logical replication access control patches
Here is a patch set to refine various access control settings in logical
replication. Currently, you need to be replication or superuser for
most things, and the goal of these patches is to allow ordinary users
equipped with explicit privileges to do most things. (Btw., current
documentation is here:
https://www.postgresql.org/docs/devel/static/logical-replication-security.html)
0001 Refine rules for altering publication owner
No conceptual changes here, just some fixes to allow altering
publication owner in more cases.
0002 Add PUBLICATION privilege
Add a new privilege kind to tables to determine whether they can be
added to a publication.
0003 Add USAGE privilege for publications
This controls whether a subscription can use the publication.
There is an open issue with this patch: Since the walsender reads
system catalogs according to what it is currently streaming, you can't
grant this privilege after a subscription has already tried to connect
and failed, because the grant will only appear in the "future" of the
stream. (You can drop and recreate the subscription, as the test
shows.) This might need some snapshot trickery around the aclcheck call.
0004 Add CREATE SUBSCRIPTION privilege on databases
New privilege to allow creating a subscription, currently restricted to
superuser.
(We could also add a CREATE PUBLICATION privilege for symmetry.
Currently, publications use the CREATE privilege that schemas also use.)
0005 Add subscription apply worker privilege checks
Makes apply workers check privileges on tables before writing to them.
Currently, all subscription owners are superuser, but 0004 proposes to
change that.
0006 Change logical replication pg_hba.conf use
No longer use the "replication" keyword in pg_hba.conf for logical
replication. Use the normal database entries instead.
Relates to
/messages/by-id/CAB7nPqRf8eOv15SPQJbC1npJoDWTNPMTNp6AvMN-XWwB53h2Cg@mail.gmail.com
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Refine-rules-for-altering-publication-owner.patchtext/x-patch; name=0001-Refine-rules-for-altering-publication-owner.patchDownload
From 544bd6e6a88ff8ced8648d4601b5e76ed2d6298c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
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 @@ <title>Description</title>
</para>
<para>
- 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 <literal>CREATE</literal> privilege on
+ the database. Also, the new owner of a <literal>FOR ALL TABLES</literal>
+ publication must be a superuser. However, a superuser can change the
+ ownership of a publication while circumventing these restrictions.
</para>
<para>
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
0002-Add-PUBLICATION-privilege.patchtext/x-patch; name=0002-Add-PUBLICATION-privilege.patchDownload
From ca0a171be99bbec31575a8693331c3d10eed5066 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 13 Feb 2017 09:52:29 -0500
Subject: [PATCH 2/6] Add PUBLICATION privilege
Adding a table to a publication requires the PUBLICATION privilege on
the table. Before, the creator of the publication also had to be the
table owner, which was less flexible.
---
doc/src/sgml/logical-replication.sgml | 7 ++++++
doc/src/sgml/ref/create_publication.sgml | 7 +++++-
doc/src/sgml/ref/grant.sgml | 15 ++++++++++--
src/backend/catalog/aclchk.c | 4 ++++
src/backend/commands/publicationcmds.c | 10 ++++----
src/backend/utils/adt/acl.c | 9 +++++++
src/bin/pg_dump/dumputils.c | 2 ++
src/bin/psql/tab-complete.c | 11 +++++----
src/include/nodes/parsenodes.h | 3 ++-
src/include/utils/acl.h | 5 ++--
src/test/regress/expected/dependency.out | 22 ++++++++---------
src/test/regress/expected/privileges.out | 40 +++++++++++++++----------------
src/test/regress/expected/publication.out | 17 +++++++++++++
src/test/regress/expected/rowsecurity.out | 34 +++++++++++++-------------
src/test/regress/sql/dependency.sql | 2 +-
src/test/regress/sql/publication.sql | 21 ++++++++++++++++
16 files changed, 145 insertions(+), 64 deletions(-)
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 7b351f2727..4889f3e591 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -308,6 +308,13 @@ <title>Security</title>
</para>
<para>
+ To add tables to a publication, the user must have
+ the <literal>PUBLICATION</literal> privilege on the table. To create a
+ publication that publishes all tables automatically, the user must be a
+ superuser.
+ </para>
+
+ <para>
To create a subscription, the user must be a superuser.
</para>
diff --git a/doc/src/sgml/ref/create_publication.sgml b/doc/src/sgml/ref/create_publication.sgml
index 995f2bcf3c..c7691dedae 100644
--- a/doc/src/sgml/ref/create_publication.sgml
+++ b/doc/src/sgml/ref/create_publication.sgml
@@ -70,6 +70,11 @@ <title>Parameters</title>
<para>
Specifies a list of tables to add to the publication.
</para>
+
+ <para>
+ Adding a table to a publication requires
+ the <literal>PUBLICATION</literal> privilege on the table.
+ </para>
</listitem>
</varlistentry>
@@ -144,7 +149,7 @@ <title>Notes</title>
<para>
To add a table to a publication, the invoking user must have
- <command>SELECT</command> privilege on given table. The
+ <command>PUBLICATION</command> privilege on given table. The
<command>FOR ALL TABLES</command> clause requires superuser.
</para>
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index d8ca39f869..6b0fbb1ff4 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -21,7 +21,7 @@
<refsynopsisdiv>
<synopsis>
-GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
+GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | PUBLICATION }
[, ...] | ALL [ PRIVILEGES ] }
ON { [ TABLE ] <replaceable class="PARAMETER">table_name</replaceable> [, ...]
| ALL TABLES IN SCHEMA <replaceable class="PARAMETER">schema_name</replaceable> [, ...] }
@@ -276,6 +276,16 @@ <title>GRANT on Database Objects</title>
</varlistentry>
<varlistentry>
+ <term>PUBLICATION</term>
+ <listitem>
+ <para>
+ Allows the use of the specified table in a publication. (See the
+ <xref linkend="sql-createpublication"> statement.)
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term>CREATE</term>
<listitem>
<para>
@@ -531,12 +541,13 @@ <title>Notes</title>
D -- TRUNCATE
x -- REFERENCES
t -- TRIGGER
+ p -- PUBLICATION
X -- EXECUTE
U -- USAGE
C -- CREATE
c -- CONNECT
T -- TEMPORARY
- arwdDxt -- ALL PRIVILEGES (for tables, varies for other objects)
+ arwdDxtp -- ALL PRIVILEGES (for tables, varies for other objects)
* -- grant option for preceding privilege
/yyyy -- role that granted this privilege
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 79b7fd5370..d8579e6a55 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -3223,6 +3223,8 @@ string_to_privilege(const char *privname)
return ACL_CREATE_TEMP;
if (strcmp(privname, "connect") == 0)
return ACL_CONNECT;
+ if (strcmp(privname, "publication") == 0)
+ return ACL_PUBLICATION;
if (strcmp(privname, "rule") == 0)
return 0; /* ignore old RULE privileges */
ereport(ERROR,
@@ -3260,6 +3262,8 @@ privilege_to_string(AclMode privilege)
return "TEMP";
case ACL_CONNECT:
return "CONNECT";
+ case ACL_PUBLICATION:
+ return "PUBLICATION";
default:
elog(ERROR, "unrecognized privilege: %d", (int) privilege);
}
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 0e4eb0726d..258f3d7ae5 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -605,12 +605,14 @@ PublicationAddTables(Oid pubid, List *rels, bool if_not_exists,
foreach(lc, rels)
{
Relation rel = (Relation) lfirst(lc);
+ AclResult aclresult;
ObjectAddress obj;
- /* Must be owner of the table or superuser. */
- if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
- aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
- RelationGetRelationName(rel));
+ aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(), ACL_PUBLICATION);
+ if (aclresult != ACLCHECK_OK)
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_CLASS,
+ RelationGetRelationName(rel));
obj = publication_add_relation(pubid, rel, if_not_exists);
if (stmt)
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 96ac1dfefd..079f9352fe 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -315,6 +315,9 @@ aclparse(const char *s, AclItem *aip)
case ACL_CONNECT_CHR:
read = ACL_CONNECT;
break;
+ case ACL_PUBLICATION_CHR:
+ read = ACL_PUBLICATION;
+ break;
case 'R': /* ignore old RULE privileges */
read = 0;
break;
@@ -1626,6 +1629,8 @@ convert_priv_string(text *priv_type_text)
return ACL_CREATE_TEMP;
if (pg_strcasecmp(priv_type, "CONNECT") == 0)
return ACL_CONNECT;
+ if (pg_strcasecmp(priv_type, "PUBLICATION") == 0)
+ return ACL_PUBLICATION;
if (pg_strcasecmp(priv_type, "RULE") == 0)
return 0; /* ignore old RULE privileges */
@@ -1722,6 +1727,8 @@ convert_aclright_to_string(int aclright)
return "TEMPORARY";
case ACL_CONNECT:
return "CONNECT";
+ case ACL_PUBLICATION:
+ return "PUBLICATION";
default:
elog(ERROR, "unrecognized aclright: %d", aclright);
return NULL;
@@ -2033,6 +2040,8 @@ convert_table_priv_string(text *priv_type_text)
{"REFERENCES WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_REFERENCES)},
{"TRIGGER", ACL_TRIGGER},
{"TRIGGER WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_TRIGGER)},
+ {"PUBLICATION", ACL_PUBLICATION},
+ {"PUBLICATION WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_PUBLICATION)},
{"RULE", 0}, /* ignore old RULE privileges */
{"RULE WITH GRANT OPTION", 0},
{NULL, 0}
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index b41f2b9125..75bb7f2c2c 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -502,6 +502,8 @@ do { \
/* table only */
CONVERT_PRIV('a', "INSERT");
CONVERT_PRIV('x', "REFERENCES");
+ if (remoteVersion >= 100000)
+ CONVERT_PRIV('p', "PUBLICATION");
/* rest are not applicable to columns */
if (subname == NULL)
{
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 94814c20d0..59519f068a 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -304,9 +304,9 @@ do { \
COMPLETE_WITH_LIST(list); \
} while (0)
-#define COMPLETE_WITH_LIST10(s1, s2, s3, s4, s5, s6, s7, s8, s9, s10) \
+#define COMPLETE_WITH_LIST11(s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11) \
do { \
- static const char *const list[] = { s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, NULL }; \
+ static const char *const list[] = { s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, NULL }; \
COMPLETE_WITH_LIST(list); \
} while (0)
@@ -2641,8 +2641,8 @@ psql_completion(const char *text, int start, int end)
* to grantable privileges (can't grant roles)
*/
if (HeadMatches3("ALTER","DEFAULT","PRIVILEGES"))
- COMPLETE_WITH_LIST10("SELECT", "INSERT", "UPDATE",
- "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER",
+ COMPLETE_WITH_LIST11("SELECT", "INSERT", "UPDATE",
+ "DELETE", "TRUNCATE", "REFERENCES", "PUBLICATION", "TRIGGER",
"EXECUTE", "USAGE", "ALL");
else
COMPLETE_WITH_QUERY(Query_for_list_of_roles
@@ -2652,6 +2652,7 @@ psql_completion(const char *text, int start, int end)
" UNION SELECT 'DELETE'"
" UNION SELECT 'TRUNCATE'"
" UNION SELECT 'REFERENCES'"
+ " UNION SELECT 'PUBLICATION'"
" UNION SELECT 'TRIGGER'"
" UNION SELECT 'CREATE'"
" UNION SELECT 'CONNECT'"
@@ -2666,7 +2667,7 @@ psql_completion(const char *text, int start, int end)
*/
else if (TailMatches2("GRANT|REVOKE", MatchAny))
{
- if (TailMatches1("SELECT|INSERT|UPDATE|DELETE|TRUNCATE|REFERENCES|TRIGGER|CREATE|CONNECT|TEMPORARY|TEMP|EXECUTE|USAGE|ALL"))
+ if (TailMatches1("SELECT|INSERT|UPDATE|DELETE|TRUNCATE|REFERENCES|PUBLICATION|TRIGGER|CREATE|CONNECT|TEMPORARY|TEMP|EXECUTE|USAGE|ALL"))
COMPLETE_WITH_CONST("ON");
else if (TailMatches2("GRANT", MatchAny))
COMPLETE_WITH_CONST("TO");
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 5afc3ebea0..e0e94dd06b 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -75,7 +75,8 @@ typedef uint32 AclMode; /* a bitmask of privilege bits */
#define ACL_CREATE (1<<9) /* for namespaces and databases */
#define ACL_CREATE_TEMP (1<<10) /* for databases */
#define ACL_CONNECT (1<<11) /* for databases */
-#define N_ACL_RIGHTS 12 /* 1 plus the last 1<<x */
+#define ACL_PUBLICATION (1<<12)
+#define N_ACL_RIGHTS 13 /* 1 plus the last 1<<x */
#define ACL_NO_RIGHTS 0
/* Currently, SELECT ... FOR [KEY] UPDATE/SHARE requires UPDATE privileges */
#define ACL_SELECT_FOR_UPDATE ACL_UPDATE
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 0d118525c9..a34d8126b7 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -139,15 +139,16 @@ typedef ArrayType Acl;
#define ACL_CREATE_CHR 'C'
#define ACL_CREATE_TEMP_CHR 'T'
#define ACL_CONNECT_CHR 'c'
+#define ACL_PUBLICATION_CHR 'p'
/* string holding all privilege code chars, in order by bitmask position */
-#define ACL_ALL_RIGHTS_STR "arwdDxtXUCTc"
+#define ACL_ALL_RIGHTS_STR "arwdDxtXUCTcp"
/*
* Bitmasks defining "all rights" for each supported object type
*/
#define ACL_ALL_RIGHTS_COLUMN (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_REFERENCES)
-#define ACL_ALL_RIGHTS_RELATION (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_TRUNCATE|ACL_REFERENCES|ACL_TRIGGER)
+#define ACL_ALL_RIGHTS_RELATION (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_TRUNCATE|ACL_REFERENCES|ACL_TRIGGER|ACL_PUBLICATION)
#define ACL_ALL_RIGHTS_SEQUENCE (ACL_USAGE|ACL_SELECT|ACL_UPDATE)
#define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP|ACL_CONNECT)
#define ACL_ALL_RIGHTS_FDW (ACL_USAGE)
diff --git a/src/test/regress/expected/dependency.out b/src/test/regress/expected/dependency.out
index 8e50f8ffbb..33b11c5732 100644
--- a/src/test/regress/expected/dependency.out
+++ b/src/test/regress/expected/dependency.out
@@ -19,7 +19,7 @@ DETAIL: privileges for table deptest
REVOKE SELECT ON deptest FROM GROUP regress_dep_group;
DROP GROUP regress_dep_group;
-- can't drop the user if we revoke the privileges partially
-REVOKE SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON deptest FROM regress_dep_user;
+REVOKE SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, PUBLICATION ON deptest FROM regress_dep_user;
DROP USER regress_dep_user;
ERROR: role "regress_dep_user" cannot be dropped because some objects depend on it
DETAIL: privileges for table deptest
@@ -63,21 +63,21 @@ CREATE TABLE deptest (a serial primary key, b text);
GRANT ALL ON deptest1 TO regress_dep_user2;
RESET SESSION AUTHORIZATION;
\z deptest1
- Access privileges
- Schema | Name | Type | Access privileges | Column privileges | Policies
---------+----------+-------+----------------------------------------------------+-------------------+----------
- public | deptest1 | table | regress_dep_user0=arwdDxt/regress_dep_user0 +| |
- | | | regress_dep_user1=a*r*w*d*D*x*t*/regress_dep_user0+| |
- | | | regress_dep_user2=arwdDxt/regress_dep_user1 | |
+ Access privileges
+ Schema | Name | Type | Access privileges | Column privileges | Policies
+--------+----------+-------+------------------------------------------------------+-------------------+----------
+ public | deptest1 | table | regress_dep_user0=arwdDxtp/regress_dep_user0 +| |
+ | | | regress_dep_user1=a*r*w*d*D*x*t*p*/regress_dep_user0+| |
+ | | | regress_dep_user2=arwdDxtp/regress_dep_user1 | |
(1 row)
DROP OWNED BY regress_dep_user1;
-- all grants revoked
\z deptest1
- Access privileges
- Schema | Name | Type | Access privileges | Column privileges | Policies
---------+----------+-------+---------------------------------------------+-------------------+----------
- public | deptest1 | table | regress_dep_user0=arwdDxt/regress_dep_user0 | |
+ Access privileges
+ Schema | Name | Type | Access privileges | Column privileges | Policies
+--------+----------+-------+----------------------------------------------+-------------------+----------
+ public | deptest1 | table | regress_dep_user0=arwdDxtp/regress_dep_user0 | |
(1 row)
-- table was dropped
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index f66b4432a1..fa47c3d0b3 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1487,38 +1487,38 @@ set session role regress_user4;
grant select on dep_priv_test to regress_user5;
\dp dep_priv_test
Access privileges
- Schema | Name | Type | Access privileges | Column privileges | Policies
---------+---------------+-------+-------------------------------------+-------------------+----------
- public | dep_priv_test | table | regress_user1=arwdDxt/regress_user1+| |
- | | | regress_user2=r*/regress_user1 +| |
- | | | regress_user3=r*/regress_user1 +| |
- | | | regress_user4=r*/regress_user2 +| |
- | | | regress_user4=r*/regress_user3 +| |
- | | | regress_user5=r/regress_user4 | |
+ Schema | Name | Type | Access privileges | Column privileges | Policies
+--------+---------------+-------+--------------------------------------+-------------------+----------
+ public | dep_priv_test | table | regress_user1=arwdDxtp/regress_user1+| |
+ | | | regress_user2=r*/regress_user1 +| |
+ | | | regress_user3=r*/regress_user1 +| |
+ | | | regress_user4=r*/regress_user2 +| |
+ | | | regress_user4=r*/regress_user3 +| |
+ | | | regress_user5=r/regress_user4 | |
(1 row)
set session role regress_user2;
revoke select on dep_priv_test from regress_user4 cascade;
\dp dep_priv_test
Access privileges
- Schema | Name | Type | Access privileges | Column privileges | Policies
---------+---------------+-------+-------------------------------------+-------------------+----------
- public | dep_priv_test | table | regress_user1=arwdDxt/regress_user1+| |
- | | | regress_user2=r*/regress_user1 +| |
- | | | regress_user3=r*/regress_user1 +| |
- | | | regress_user4=r*/regress_user3 +| |
- | | | regress_user5=r/regress_user4 | |
+ Schema | Name | Type | Access privileges | Column privileges | Policies
+--------+---------------+-------+--------------------------------------+-------------------+----------
+ public | dep_priv_test | table | regress_user1=arwdDxtp/regress_user1+| |
+ | | | regress_user2=r*/regress_user1 +| |
+ | | | regress_user3=r*/regress_user1 +| |
+ | | | regress_user4=r*/regress_user3 +| |
+ | | | regress_user5=r/regress_user4 | |
(1 row)
set session role regress_user3;
revoke select on dep_priv_test from regress_user4 cascade;
\dp dep_priv_test
Access privileges
- Schema | Name | Type | Access privileges | Column privileges | Policies
---------+---------------+-------+-------------------------------------+-------------------+----------
- public | dep_priv_test | table | regress_user1=arwdDxt/regress_user1+| |
- | | | regress_user2=r*/regress_user1 +| |
- | | | regress_user3=r*/regress_user1 | |
+ Schema | Name | Type | Access privileges | Column privileges | Policies
+--------+---------------+-------+--------------------------------------+-------------------+----------
+ public | dep_priv_test | table | regress_user1=arwdDxtp/regress_user1+| |
+ | | | regress_user2=r*/regress_user1 +| |
+ | | | regress_user3=r*/regress_user1 | |
(1 row)
set session role regress_user1;
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 448e708c08..e63612e0d5 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -140,6 +140,23 @@ Publications:
"testpib_ins_trunct"
"testpub_fortbl"
+-- permissions
+SET ROLE regress_publication_user2;
+CREATE PUBLICATION testpub2; -- fail
+ERROR: permission denied for database regression
+SET ROLE regress_publication_user;
+GRANT CREATE ON DATABASE regression TO regress_publication_user2;
+SET ROLE regress_publication_user2;
+CREATE PUBLICATION testpub2; -- ok
+ALTER PUBLICATION testpub2 ADD TABLE testpub_tbl1; -- fail
+ERROR: permission denied for relation testpub_tbl1
+SET ROLE regress_publication_user;
+GRANT PUBLICATION ON testpub_tbl1 TO regress_publication_user2;
+SET ROLE regress_publication_user2;
+ALTER PUBLICATION testpub2 ADD TABLE testpub_tbl1; -- ok
+DROP PUBLICATION testpub2;
+SET ROLE regress_publication_user;
+REVOKE CREATE ON DATABASE regression FROM regress_publication_user2;
DROP VIEW testpub_view;
DROP TABLE testpub_tbl1;
\dRp+ testpub_default
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 7bf29368d0..ccc22b7e34 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -93,23 +93,23 @@ CREATE POLICY p2r ON document AS RESTRICTIVE TO regress_rls_dave
CREATE POLICY p1r ON document AS RESTRICTIVE TO regress_rls_dave
USING (cid <> 44);
\dp
- Access privileges
- Schema | Name | Type | Access privileges | Column privileges | Policies
---------------------+----------+-------+---------------------------------------------+-------------------+--------------------------------------------
- regress_rls_schema | category | table | regress_rls_alice=arwdDxt/regress_rls_alice+| |
- | | | =arwdDxt/regress_rls_alice | |
- regress_rls_schema | document | table | regress_rls_alice=arwdDxt/regress_rls_alice+| | p1: +
- | | | =arwdDxt/regress_rls_alice | | (u): (dlevel <= ( SELECT uaccount.seclv +
- | | | | | FROM uaccount +
- | | | | | WHERE (uaccount.pguser = CURRENT_USER)))+
- | | | | | p2r (RESTRICTIVE): +
- | | | | | (u): ((cid <> 44) AND (cid < 50)) +
- | | | | | to: regress_rls_dave +
- | | | | | p1r (RESTRICTIVE): +
- | | | | | (u): (cid <> 44) +
- | | | | | to: regress_rls_dave
- regress_rls_schema | uaccount | table | regress_rls_alice=arwdDxt/regress_rls_alice+| |
- | | | =r/regress_rls_alice | |
+ Access privileges
+ Schema | Name | Type | Access privileges | Column privileges | Policies
+--------------------+----------+-------+----------------------------------------------+-------------------+--------------------------------------------
+ regress_rls_schema | category | table | regress_rls_alice=arwdDxtp/regress_rls_alice+| |
+ | | | =arwdDxtp/regress_rls_alice | |
+ regress_rls_schema | document | table | regress_rls_alice=arwdDxtp/regress_rls_alice+| | p1: +
+ | | | =arwdDxtp/regress_rls_alice | | (u): (dlevel <= ( SELECT uaccount.seclv +
+ | | | | | FROM uaccount +
+ | | | | | WHERE (uaccount.pguser = CURRENT_USER)))+
+ | | | | | p2r (RESTRICTIVE): +
+ | | | | | (u): ((cid <> 44) AND (cid < 50)) +
+ | | | | | to: regress_rls_dave +
+ | | | | | p1r (RESTRICTIVE): +
+ | | | | | (u): (cid <> 44) +
+ | | | | | to: regress_rls_dave
+ regress_rls_schema | uaccount | table | regress_rls_alice=arwdDxtp/regress_rls_alice+| |
+ | | | =r/regress_rls_alice | |
(3 rows)
\d document
diff --git a/src/test/regress/sql/dependency.sql b/src/test/regress/sql/dependency.sql
index f5c45e4666..2bdd51532c 100644
--- a/src/test/regress/sql/dependency.sql
+++ b/src/test/regress/sql/dependency.sql
@@ -21,7 +21,7 @@ CREATE TABLE deptest (f1 serial primary key, f2 text);
DROP GROUP regress_dep_group;
-- can't drop the user if we revoke the privileges partially
-REVOKE SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON deptest FROM regress_dep_user;
+REVOKE SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, PUBLICATION ON deptest FROM regress_dep_user;
DROP USER regress_dep_user;
-- now we are OK to drop him
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 4b22053b13..7b322bb7d9 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -69,6 +69,27 @@ CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1;
\d+ testpub_tbl1
+-- permissions
+SET ROLE regress_publication_user2;
+CREATE PUBLICATION testpub2; -- fail
+
+SET ROLE regress_publication_user;
+GRANT CREATE ON DATABASE regression TO regress_publication_user2;
+SET ROLE regress_publication_user2;
+CREATE PUBLICATION testpub2; -- ok
+
+ALTER PUBLICATION testpub2 ADD TABLE testpub_tbl1; -- fail
+
+SET ROLE regress_publication_user;
+GRANT PUBLICATION ON testpub_tbl1 TO regress_publication_user2;
+SET ROLE regress_publication_user2;
+ALTER PUBLICATION testpub2 ADD TABLE testpub_tbl1; -- ok
+
+DROP PUBLICATION testpub2;
+
+SET ROLE regress_publication_user;
+REVOKE CREATE ON DATABASE regression FROM regress_publication_user2;
+
DROP VIEW testpub_view;
DROP TABLE testpub_tbl1;
--
2.11.1
0003-Add-USAGE-privilege-for-publications.patchtext/x-patch; name=0003-Add-USAGE-privilege-for-publications.patchDownload
From dacee128d165fb1194ec3f1eef612ce410e986c5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 2 Nov 2016 12:00:00 -0400
Subject: [PATCH 3/6] Add USAGE privilege for publications
Add pg_publication.pubacl column and associated infrastructure so that
publications can have privileges. USAGE privilege on the publication is
now required for a connecting subscription to be able to use it.
Previously, any connecting user could use any publication, which was not
unreasonable because that user needs to have the REPLICATION attribute,
which is pretty powerful anyway, but we might want to move away from
that, and this gives finer control.
---
doc/src/sgml/catalogs.sgml | 12 ++
doc/src/sgml/func.sgml | 27 ++++
doc/src/sgml/logical-replication.sgml | 5 +
doc/src/sgml/ref/grant.sgml | 8 ++
doc/src/sgml/ref/revoke.sgml | 6 +
src/backend/catalog/aclchk.c | 213 ++++++++++++++++++++++++++++
src/backend/commands/event_trigger.c | 1 +
src/backend/commands/publicationcmds.c | 2 +
src/backend/parser/gram.y | 8 ++
src/backend/replication/pgoutput/pgoutput.c | 8 ++
src/backend/utils/adt/acl.c | 200 ++++++++++++++++++++++++++
src/bin/pg_dump/dumputils.c | 2 +
src/bin/pg_dump/pg_dump.c | 60 ++++++--
src/bin/pg_dump/pg_dump.h | 4 +
src/bin/psql/describe.c | 8 +-
src/bin/psql/tab-complete.c | 1 +
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_proc.h | 13 ++
src/include/catalog/pg_publication.h | 6 +-
src/include/nodes/parsenodes.h | 1 +
src/include/utils/acl.h | 4 +
src/test/regress/expected/publication.out | 34 ++---
src/test/regress/sql/publication.sql | 2 +
src/test/subscription/t/003_privileges.pl | 72 ++++++++++
24 files changed, 670 insertions(+), 29 deletions(-)
create mode 100644 src/test/subscription/t/003_privileges.pl
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 96cb9185c2..4537fec8eb 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -5366,6 +5366,18 @@ <title><structname>pg_publication</structname> Columns</title>
<entry>If true, <command>DELETE</command> operations are replicated for
tables in the publication.</entry>
</row>
+
+ <row>
+ <entry><structfield>pubacl</structfield></entry>
+ <entry><type>aclitem[]</type></entry>
+ <entry></entry>
+ <entry>
+ Access privileges; see
+ <xref linkend="sql-grant"> and
+ <xref linkend="sql-revoke">
+ for details
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index d7738b18b7..81320b1941 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -15859,6 +15859,21 @@ <title>Access Privilege Inquiry Functions</title>
<entry>does current user have privilege for language</entry>
</row>
<row>
+ <entry><literal><function>has_publication_privilege</function>(<parameter>user</parameter>,
+ <parameter>publication</parameter>,
+ <parameter>privilege</parameter>)</literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>does user have privilege for publication</entry>
+ </row>
+ <row>
+ <entry><literal><function>has_publication_privilege</function>(<parameter>publication</parameter>,
+ <parameter>privilege</parameter>)</literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>does current user have privilege for publication</entry>
+ </row>
+ <row>
<entry><literal><function>has_schema_privilege</function>(<parameter>user</parameter>,
<parameter>schema</parameter>,
<parameter>privilege</parameter>)</literal>
@@ -15992,6 +16007,9 @@ <title>Access Privilege Inquiry Functions</title>
<primary>has_language_privilege</primary>
</indexterm>
<indexterm>
+ <primary>has_publication_privilege</primary>
+ </indexterm>
+ <indexterm>
<primary>has_schema_privilege</primary>
</indexterm>
<indexterm>
@@ -16136,6 +16154,15 @@ <title>Access Privilege Inquiry Functions</title>
</para>
<para>
+ <function>has_publication_privilege</function> checks whether a user
+ can access a publication in a particular way.
+ Its argument possibilities
+ are analogous to <function>has_table_privilege</function>.
+ The desired access privilege type must evaluate to
+ <literal>USAGE</literal>.
+ </para>
+
+ <para>
<function>has_schema_privilege</function> checks whether a user
can access a schema in a particular way.
Its argument possibilities
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 4889f3e591..b6636d76b1 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -319,6 +319,11 @@ <title>Security</title>
</para>
<para>
+ To use a publication, the remote user of a subscription connection must
+ have the <literal>USAGE</literal> privilege on the publication.
+ </para>
+
+ <para>
The subscription apply process will run in the local database with the
privileges of a superuser.
</para>
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index 6b0fbb1ff4..996d183561 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -67,6 +67,10 @@
ON LARGE OBJECT <replaceable class="PARAMETER">loid</replaceable> [, ...]
TO <replaceable class="PARAMETER">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
+GRANT { USAGE | ALL [ PRIVILEGES ] }
+ ON PUBLICATION <replaceable>publication_name</replaceable> [, ...]
+ TO <replaceable class="PARAMETER">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
+
GRANT { { CREATE | USAGE } [, ...] | ALL [ PRIVILEGES ] }
ON SCHEMA <replaceable>schema_name</replaceable> [, ...]
TO <replaceable class="PARAMETER">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
@@ -378,6 +382,10 @@ <title>GRANT on Database Objects</title>
tables using the server, and also to create, alter, or drop their own
user's user mappings associated with that server.
</para>
+ <para>
+ For publications, this privilege allows a subscription to use the
+ publication.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/revoke.sgml b/doc/src/sgml/ref/revoke.sgml
index fc00129620..6ba532dff8 100644
--- a/doc/src/sgml/ref/revoke.sgml
+++ b/doc/src/sgml/ref/revoke.sgml
@@ -88,6 +88,12 @@
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
+ { USAGE | ALL [ PRIVILEGES ] }
+ ON PUBLICATION <replaceable>publication_name</replaceable> [, ...]
+ FROM { [ GROUP ] <replaceable class="PARAMETER">role_name</replaceable> | PUBLIC } [, ...]
+ [ CASCADE | RESTRICT ]
+
+REVOKE [ GRANT OPTION FOR ]
{ { CREATE | USAGE } [, ...] | ALL [ PRIVILEGES ] }
ON SCHEMA <replaceable>schema_name</replaceable> [, ...]
FROM { [ GROUP ] <replaceable class="PARAMETER">role_name</replaceable> | PUBLIC } [, ...]
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index d8579e6a55..20ab6d5b0d 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -109,6 +109,7 @@ static void ExecGrant_Function(InternalGrant *grantStmt);
static void ExecGrant_Language(InternalGrant *grantStmt);
static void ExecGrant_Largeobject(InternalGrant *grantStmt);
static void ExecGrant_Namespace(InternalGrant *grantStmt);
+static void ExecGrant_Publication(InternalGrant *grantStmt);
static void ExecGrant_Tablespace(InternalGrant *grantStmt);
static void ExecGrant_Type(InternalGrant *grantStmt);
@@ -283,6 +284,9 @@ restrict_and_check_grant(bool is_grant, AclMode avail_goptions, bool all_privs,
case ACL_KIND_TYPE:
whole_mask = ACL_ALL_RIGHTS_TYPE;
break;
+ case ACL_KIND_PUBLICATION:
+ whole_mask = ACL_ALL_RIGHTS_PUBLICATION;
+ break;
default:
elog(ERROR, "unrecognized object kind: %d", objkind);
/* not reached, but keep compiler quiet */
@@ -497,6 +501,10 @@ ExecuteGrantStmt(GrantStmt *stmt)
all_privileges = ACL_ALL_RIGHTS_FOREIGN_SERVER;
errormsg = gettext_noop("invalid privilege type %s for foreign server");
break;
+ case ACL_OBJECT_PUBLICATION:
+ all_privileges = ACL_ALL_RIGHTS_PUBLICATION;
+ errormsg = gettext_noop("invalid privilege type %s for publication");
+ break;
default:
elog(ERROR, "unrecognized GrantStmt.objtype: %d",
(int) stmt->objtype);
@@ -594,6 +602,9 @@ ExecGrantStmt_oids(InternalGrant *istmt)
case ACL_OBJECT_NAMESPACE:
ExecGrant_Namespace(istmt);
break;
+ case ACL_OBJECT_PUBLICATION:
+ ExecGrant_Publication(istmt);
+ break;
case ACL_OBJECT_TABLESPACE:
ExecGrant_Tablespace(istmt);
break;
@@ -737,6 +748,15 @@ objectNamesToOids(GrantObjectType objtype, List *objnames)
objects = lappend_oid(objects, srvid);
}
break;
+ case ACL_OBJECT_PUBLICATION:
+ foreach(cell, objnames)
+ {
+ char *pubname = strVal(lfirst(cell));
+ Oid pubid = get_publication_oid(pubname, false);
+
+ objects = lappend_oid(objects, pubid);
+ }
+ break;
default:
elog(ERROR, "unrecognized GrantStmt.objtype: %d",
(int) objtype);
@@ -2937,6 +2957,126 @@ ExecGrant_Namespace(InternalGrant *istmt)
}
static void
+ExecGrant_Publication(InternalGrant *istmt)
+{
+ Relation relation;
+ ListCell *cell;
+
+ if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
+ istmt->privileges = ACL_ALL_RIGHTS_PUBLICATION;
+
+ relation = heap_open(PublicationRelationId, RowExclusiveLock);
+
+ foreach(cell, istmt->objects)
+ {
+ Oid pubId = lfirst_oid(cell);
+ Form_pg_publication pg_publication_tuple;
+ Datum aclDatum;
+ bool isNull;
+ AclMode avail_goptions;
+ AclMode this_privileges;
+ Acl *old_acl;
+ Acl *new_acl;
+ Oid grantorId;
+ Oid ownerId;
+ HeapTuple newtuple;
+ Datum values[Natts_pg_publication];
+ bool nulls[Natts_pg_publication];
+ bool replaces[Natts_pg_publication];
+ int noldmembers;
+ int nnewmembers;
+ Oid *oldmembers;
+ Oid *newmembers;
+ HeapTuple tuple;
+
+ /* Search syscache for pg_publication */
+ tuple = SearchSysCache1(PUBLICATIONOID, ObjectIdGetDatum(pubId));
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "cache lookup failed for publication %u", pubId);
+
+ pg_publication_tuple = (Form_pg_publication) GETSTRUCT(tuple);
+
+ /*
+ * Get owner ID and working copy of existing ACL. If there's no ACL,
+ * substitute the proper default.
+ */
+ ownerId = pg_publication_tuple->pubowner;
+ aclDatum = heap_getattr(tuple, Anum_pg_publication_pubacl,
+ RelationGetDescr(relation), &isNull);
+ if (isNull)
+ {
+ old_acl = acldefault(ACL_OBJECT_PUBLICATION, ownerId);
+ /* There are no old member roles according to the catalogs */
+ noldmembers = 0;
+ oldmembers = NULL;
+ }
+ else
+ {
+ old_acl = DatumGetAclPCopy(aclDatum);
+ /* Get the roles mentioned in the existing ACL */
+ noldmembers = aclmembers(old_acl, &oldmembers);
+ }
+
+ /* Determine ID to do the grant as, and available grant options */
+ select_best_grantor(GetUserId(), istmt->privileges,
+ old_acl, ownerId,
+ &grantorId, &avail_goptions);
+
+ /*
+ * Restrict the privileges to what we can actually grant, and emit the
+ * standards-mandated warning and error messages.
+ */
+ this_privileges =
+ restrict_and_check_grant(istmt->is_grant, avail_goptions,
+ istmt->all_privs, istmt->privileges,
+ pubId, grantorId, ACL_KIND_PUBLICATION,
+ NameStr(pg_publication_tuple->pubname),
+ 0, NULL);
+
+ /*
+ * Generate new ACL.
+ */
+ new_acl = merge_acl_with_grant(old_acl, istmt->is_grant,
+ istmt->grant_option, istmt->behavior,
+ istmt->grantees, this_privileges,
+ grantorId, ownerId);
+
+ /*
+ * We need the members of both old and new ACLs so we can correct the
+ * shared dependency information.
+ */
+ nnewmembers = aclmembers(new_acl, &newmembers);
+
+ /* finished building new ACL value, now insert it */
+ MemSet(values, 0, sizeof(values));
+ MemSet(nulls, false, sizeof(nulls));
+ MemSet(replaces, false, sizeof(replaces));
+
+ replaces[Anum_pg_publication_pubacl - 1] = true;
+ values[Anum_pg_publication_pubacl - 1] = PointerGetDatum(new_acl);
+
+ newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
+ nulls, replaces);
+
+ CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
+
+ /* Update the shared dependency ACL info */
+ updateAclDependencies(PublicationRelationId, pubId, 0,
+ ownerId,
+ noldmembers, oldmembers,
+ nnewmembers, newmembers);
+
+ ReleaseSysCache(tuple);
+ pfree(new_acl);
+
+ /* prevent error when processing duplicate objects */
+ CommandCounterIncrement();
+ }
+
+ heap_close(relation, RowExclusiveLock);
+}
+
+static void
ExecGrant_Tablespace(InternalGrant *istmt)
{
Relation relation;
@@ -4197,6 +4337,67 @@ pg_foreign_server_aclmask(Oid srv_oid, Oid roleid,
}
/*
+ * Exported routine for examining a user's privileges for a publication.
+ */
+AclMode
+pg_publication_aclmask(Oid pub_oid, Oid roleid,
+ AclMode mask, AclMaskHow how)
+{
+ AclMode result;
+ HeapTuple tuple;
+ Datum aclDatum;
+ bool isNull;
+ Acl *acl;
+ Oid ownerId;
+
+ Form_pg_publication pubForm;
+
+ /* Bypass permission checks for superusers */
+ if (superuser_arg(roleid))
+ return mask;
+
+ /*
+ * Must get the publication's tuple from pg_publication
+ */
+ tuple = SearchSysCache1(PUBLICATIONOID, ObjectIdGetDatum(pub_oid));
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("publication with OID %u does not exist",
+ pub_oid)));
+ pubForm = (Form_pg_publication) GETSTRUCT(tuple);
+
+ /*
+ * Normal case: get the publication's ACL from pg_publication
+ */
+ ownerId = pubForm->pubowner;
+
+ aclDatum = SysCacheGetAttr(PUBLICATIONOID, tuple,
+ Anum_pg_publication_pubacl, &isNull);
+ if (isNull)
+ {
+ /* No ACL, so build default ACL */
+ acl = acldefault(ACL_OBJECT_PUBLICATION, ownerId);
+ aclDatum = (Datum) 0;
+ }
+ else
+ {
+ /* detoast rel's ACL if necessary */
+ acl = DatumGetAclP(aclDatum);
+ }
+
+ result = aclmask(acl, roleid, ownerId, mask, how);
+
+ /* if we have a detoasted copy, free it */
+ if (acl && (Pointer) acl != DatumGetPointer(aclDatum))
+ pfree(acl);
+
+ ReleaseSysCache(tuple);
+
+ return result;
+}
+
+/*
* Exported routine for examining a user's privileges for a type.
*/
AclMode
@@ -4507,6 +4708,18 @@ pg_foreign_server_aclcheck(Oid srv_oid, Oid roleid, AclMode mode)
}
/*
+ * Exported routine for checking a user's access privileges to a publication
+ */
+AclResult
+pg_publication_aclcheck(Oid pub_oid, Oid roleid, AclMode mode)
+{
+ if (pg_publication_aclmask(pub_oid, roleid, mode, ACLMASK_ANY) != 0)
+ return ACLCHECK_OK;
+ else
+ return ACLCHECK_NO_PRIV;
+}
+
+/*
* Exported routine for checking a user's access privileges to a type
*/
AclResult
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 346b347ae1..7438cfa59a 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -1199,6 +1199,7 @@ EventTriggerSupportsGrantObjectType(GrantObjectType objtype)
case ACL_OBJECT_LANGUAGE:
case ACL_OBJECT_LARGEOBJECT:
case ACL_OBJECT_NAMESPACE:
+ case ACL_OBJECT_PUBLICATION:
case ACL_OBJECT_TYPE:
return true;
default:
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 258f3d7ae5..e06b4b7bdb 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -212,6 +212,8 @@ CreatePublication(CreatePublicationStmt *stmt)
values[Anum_pg_publication_pubdelete - 1] =
BoolGetDatum(publish_delete);
+ nulls[Anum_pg_publication_pubacl - 1] = true;
+
tup = heap_form_tuple(RelationGetDescr(rel), values, nulls);
/* Insert tuple into catalog. */
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 07cc81ee76..2f05264e84 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -6864,6 +6864,14 @@ privilege_target:
n->objs = $3;
$$ = n;
}
+ | PUBLICATION name_list
+ {
+ PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
+ n->targtype = ACL_TARGET_OBJECT;
+ n->objtype = ACL_OBJECT_PUBLICATION;
+ n->objs = $2;
+ $$ = n;
+ }
| SCHEMA name_list
{
PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 0ceb4be375..54fb8fa185 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -14,6 +14,8 @@
#include "catalog/pg_publication.h"
+#include "miscadmin.h"
+
#include "replication/logical.h"
#include "replication/logicalproto.h"
#include "replication/origin.h"
@@ -398,6 +400,12 @@ LoadPublications(List *pubnames)
{
char *pubname = (char *) lfirst(lc);
Publication *pub = GetPublicationByName(pubname, false);
+ AclResult aclresult;
+
+ aclresult = pg_publication_aclcheck(pub->oid, GetUserId(), ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_PUBLICATION,
+ get_publication_name(pub->oid));
result = lappend(result, pub);
}
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 079f9352fe..e84c8d80c7 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -107,6 +107,8 @@ static Oid convert_function_name(text *functionname);
static AclMode convert_function_priv_string(text *priv_type_text);
static Oid convert_language_name(text *languagename);
static AclMode convert_language_priv_string(text *priv_type_text);
+static Oid convert_publication_name(text *publicationname);
+static AclMode convert_publication_priv_string(text *priv_type_text);
static Oid convert_schema_name(text *schemaname);
static AclMode convert_schema_priv_string(text *priv_type_text);
static Oid convert_server_name(text *servername);
@@ -795,6 +797,10 @@ acldefault(GrantObjectType objtype, Oid ownerId)
world_default = ACL_USAGE;
owner_default = ACL_ALL_RIGHTS_TYPE;
break;
+ case ACL_OBJECT_PUBLICATION:
+ world_default = ACL_NO_RIGHTS;
+ owner_default = ACL_ALL_RIGHTS_PUBLICATION;
+ break;
default:
elog(ERROR, "unrecognized objtype: %d", (int) objtype);
world_default = ACL_NO_RIGHTS; /* keep compiler quiet */
@@ -887,6 +893,9 @@ acldefault_sql(PG_FUNCTION_ARGS)
case 'S':
objtype = ACL_OBJECT_FOREIGN_SERVER;
break;
+ case 'p':
+ objtype = ACL_OBJECT_PUBLICATION;
+ break;
case 'T':
objtype = ACL_OBJECT_TYPE;
break;
@@ -3648,6 +3657,197 @@ convert_language_priv_string(text *priv_type_text)
/*
+ * has_publication_privilege variants
+ * These are all named "has_publication_privilege" at the SQL level.
+ * They take various combinations of publication name, publication OID,
+ * user name, user OID, or implicit user = current_user.
+ *
+ * The result is a boolean value: true if user has the indicated
+ * privilege, false if not, or NULL if object doesn't exist.
+ */
+
+/*
+ * has_publication_privilege_name_name
+ * Check user privileges on a publication given
+ * name username, text publicationname, and text priv name.
+ */
+Datum
+has_publication_privilege_name_name(PG_FUNCTION_ARGS)
+{
+ Name username = PG_GETARG_NAME(0);
+ text *publicationname = PG_GETARG_TEXT_P(1);
+ text *priv_type_text = PG_GETARG_TEXT_P(2);
+ Oid roleid;
+ Oid publicationoid;
+ AclMode mode;
+ AclResult aclresult;
+
+ roleid = get_role_oid_or_public(NameStr(*username));
+ publicationoid = convert_publication_name(publicationname);
+ mode = convert_publication_priv_string(priv_type_text);
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_publication_privilege_name
+ * Check user privileges on a publication given
+ * text publicationname and text priv name.
+ * current_user is assumed
+ */
+Datum
+has_publication_privilege_name(PG_FUNCTION_ARGS)
+{
+ text *publicationname = PG_GETARG_TEXT_P(0);
+ text *priv_type_text = PG_GETARG_TEXT_P(1);
+ Oid roleid;
+ Oid publicationoid;
+ AclMode mode;
+ AclResult aclresult;
+
+ roleid = GetUserId();
+ publicationoid = convert_publication_name(publicationname);
+ mode = convert_publication_priv_string(priv_type_text);
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_publication_privilege_name_id
+ * Check user privileges on a publication given
+ * name usename, publication oid, and text priv name.
+ */
+Datum
+has_publication_privilege_name_id(PG_FUNCTION_ARGS)
+{
+ Name username = PG_GETARG_NAME(0);
+ Oid publicationoid = PG_GETARG_OID(1);
+ text *priv_type_text = PG_GETARG_TEXT_P(2);
+ Oid roleid;
+ AclMode mode;
+ AclResult aclresult;
+
+ roleid = get_role_oid_or_public(NameStr(*username));
+ mode = convert_publication_priv_string(priv_type_text);
+
+ if (!SearchSysCacheExists1(PUBLICATIONOID, ObjectIdGetDatum(publicationoid)))
+ PG_RETURN_NULL();
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_publication_privilege_id
+ * Check user privileges on a publication given
+ * publication oid, and text priv name.
+ * current_user is assumed
+ */
+Datum
+has_publication_privilege_id(PG_FUNCTION_ARGS)
+{
+ Oid publicationoid = PG_GETARG_OID(0);
+ text *priv_type_text = PG_GETARG_TEXT_P(1);
+ Oid roleid;
+ AclMode mode;
+ AclResult aclresult;
+
+ roleid = GetUserId();
+ mode = convert_publication_priv_string(priv_type_text);
+
+ if (!SearchSysCacheExists1(PUBLICATIONOID, ObjectIdGetDatum(publicationoid)))
+ PG_RETURN_NULL();
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_publication_privilege_id_name
+ * Check user privileges on a publication given
+ * roleid, text publicationname, and text priv name.
+ */
+Datum
+has_publication_privilege_id_name(PG_FUNCTION_ARGS)
+{
+ Oid roleid = PG_GETARG_OID(0);
+ text *publicationname = PG_GETARG_TEXT_P(1);
+ text *priv_type_text = PG_GETARG_TEXT_P(2);
+ Oid publicationoid;
+ AclMode mode;
+ AclResult aclresult;
+
+ publicationoid = convert_publication_name(publicationname);
+ mode = convert_publication_priv_string(priv_type_text);
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_publication_privilege_id_id
+ * Check user privileges on a publication given
+ * roleid, publication oid, and text priv name.
+ */
+Datum
+has_publication_privilege_id_id(PG_FUNCTION_ARGS)
+{
+ Oid roleid = PG_GETARG_OID(0);
+ Oid publicationoid = PG_GETARG_OID(1);
+ text *priv_type_text = PG_GETARG_TEXT_P(2);
+ AclMode mode;
+ AclResult aclresult;
+
+ mode = convert_publication_priv_string(priv_type_text);
+
+ if (!SearchSysCacheExists1(PUBLICATIONOID, ObjectIdGetDatum(publicationoid)))
+ PG_RETURN_NULL();
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * Support routines for has_publication_privilege family.
+ */
+
+/*
+ * Given a publication name expressed as a string, look it up and return Oid
+ */
+static Oid
+convert_publication_name(text *publicationname)
+{
+ char *pubname = text_to_cstring(publicationname);
+
+ return get_publication_oid(pubname, false);
+}
+
+/*
+ * convert_publication_priv_string
+ * Convert text string to AclMode value.
+ */
+static AclMode
+convert_publication_priv_string(text *priv_type_text)
+{
+ static const priv_map publication_priv_map[] = {
+ {"USAGE", ACL_USAGE},
+ {"USAGE WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_USAGE)},
+ {NULL, 0}
+ };
+
+ return convert_any_priv_string(priv_type_text, publication_priv_map);
+}
+
+
+/*
* has_schema_privilege variants
* These are all named "has_schema_privilege" at the SQL level.
* They take various combinations of schema name, schema OID,
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 75bb7f2c2c..8b6d70f2cb 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -522,6 +522,8 @@ do { \
CONVERT_PRIV('X', "EXECUTE");
else if (strcmp(type, "LANGUAGE") == 0)
CONVERT_PRIV('U', "USAGE");
+ else if (strcmp(type, "PUBLICATION") == 0)
+ CONVERT_PRIV('U', "USAGE");
else if (strcmp(type, "SCHEMA") == 0)
{
CONVERT_PRIV('C', "CREATE");
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7364a12c25..2e5c9b068d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3327,7 +3327,11 @@ dumpPolicy(Archive *fout, PolicyInfo *polinfo)
void
getPublications(Archive *fout)
{
- PQExpBuffer query;
+ PQExpBuffer acl_subquery = createPQExpBuffer();
+ PQExpBuffer racl_subquery = createPQExpBuffer();
+ PQExpBuffer initacl_subquery = createPQExpBuffer();
+ PQExpBuffer initracl_subquery = createPQExpBuffer();
+ PQExpBuffer query = createPQExpBuffer();
PGresult *res;
PublicationInfo *pubinfo;
int i_tableoid;
@@ -3338,24 +3342,46 @@ getPublications(Archive *fout)
int i_pubinsert;
int i_pubupdate;
int i_pubdelete;
+ int i_pubacl;
+ int i_rpubacl;
+ int i_initpubacl;
+ int i_initrpubacl;
int i,
ntups;
+
if (fout->remoteVersion < 100000)
return;
- query = createPQExpBuffer();
-
- resetPQExpBuffer(query);
+ buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
+ initracl_subquery, "p.pubacl", "p.pubowner", "'p'",
+ fout->dopt->binary_upgrade);
/* Get the publications. */
appendPQExpBuffer(query,
"SELECT p.tableoid, p.oid, p.pubname, "
+ "%s AS pubacl, "
+ "%s AS rpubacl, "
+ "%s AS initpubacl, "
+ "%s AS initrpubacl, "
"(%s p.pubowner) AS rolname, "
"p.puballtables, p.pubinsert, p.pubupdate, p.pubdelete "
- "FROM pg_catalog.pg_publication p",
+ "FROM pg_catalog.pg_publication p "
+ "LEFT JOIN pg_init_privs pip ON "
+ "(p.oid = pip.objoid "
+ "AND pip.classoid = 'pg_publication'::regclass "
+ "AND pip.objsubid = 0) ",
+ acl_subquery->data,
+ racl_subquery->data,
+ initacl_subquery->data,
+ initracl_subquery->data,
username_subquery);
+ destroyPQExpBuffer(acl_subquery);
+ destroyPQExpBuffer(racl_subquery);
+ destroyPQExpBuffer(initacl_subquery);
+ destroyPQExpBuffer(initracl_subquery);
+
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
ntups = PQntuples(res);
@@ -3368,6 +3394,10 @@ getPublications(Archive *fout)
i_pubinsert = PQfnumber(res, "pubinsert");
i_pubupdate = PQfnumber(res, "pubupdate");
i_pubdelete = PQfnumber(res, "pubdelete");
+ i_pubacl = PQfnumber(res, "pubacl");
+ i_rpubacl = PQfnumber(res, "rpubacl");
+ i_initpubacl = PQfnumber(res, "initpubacl");
+ i_initrpubacl = PQfnumber(res, "initrpubacl");
pubinfo = pg_malloc(ntups * sizeof(PublicationInfo));
@@ -3388,6 +3418,10 @@ getPublications(Archive *fout)
(strcmp(PQgetvalue(res, i, i_pubupdate), "t") == 0);
pubinfo[i].pubdelete =
(strcmp(PQgetvalue(res, i, i_pubdelete), "t") == 0);
+ pubinfo[i].pubacl = pg_strdup(PQgetvalue(res, i, i_pubacl));
+ pubinfo[i].rpubacl = pg_strdup(PQgetvalue(res, i, i_rpubacl));
+ pubinfo[i].initpubacl = pg_strdup(PQgetvalue(res, i, i_initpubacl));
+ pubinfo[i].initrpubacl = pg_strdup(PQgetvalue(res, i, i_initrpubacl));
if (strlen(pubinfo[i].rolname) == 0)
write_msg(NULL, "WARNING: owner of publication \"%s\" appears to be invalid\n",
@@ -3408,6 +3442,7 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
DumpOptions *dopt = fout->dopt;
PQExpBuffer delq;
PQExpBuffer query;
+ char *qpubname;
if (dopt->dataOnly)
return;
@@ -3415,11 +3450,11 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
delq = createPQExpBuffer();
query = createPQExpBuffer();
- appendPQExpBuffer(delq, "DROP PUBLICATION %s;\n",
- fmtId(pubinfo->dobj.name));
+ qpubname = pg_strdup(fmtId(pubinfo->dobj.name));
- appendPQExpBuffer(query, "CREATE PUBLICATION %s",
- fmtId(pubinfo->dobj.name));
+ appendPQExpBuffer(delq, "DROP PUBLICATION %s;\n", qpubname);
+
+ appendPQExpBuffer(query, "CREATE PUBLICATION %s", qpubname);
if (pubinfo->puballtables)
appendPQExpBufferStr(query, " FOR ALL TABLES");
@@ -3452,6 +3487,13 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
NULL, 0,
NULL, NULL);
+ if (pubinfo->dobj.dump & DUMP_COMPONENT_ACL)
+ dumpACL(fout, pubinfo->dobj.catId, pubinfo->dobj.dumpId, "PUBLICATION",
+ qpubname, NULL, pubinfo->dobj.name,
+ NULL, pubinfo->rolname, pubinfo->pubacl, pubinfo->rpubacl,
+ pubinfo->initpubacl, pubinfo->initrpubacl);
+
+ free(qpubname);
destroyPQExpBuffer(delq);
destroyPQExpBuffer(query);
}
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index a466527ec6..533ac24ebf 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -583,6 +583,10 @@ typedef struct _PublicationInfo
bool pubinsert;
bool pubupdate;
bool pubdelete;
+ char *pubacl;
+ char *rpubacl;
+ char *initpubacl;
+ char *initrpubacl;
} PublicationInfo;
/*
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index e2e4cbcc08..7011da08fa 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -4966,7 +4966,9 @@ describePublications(const char *pattern)
printfPQExpBuffer(&buf,
"SELECT oid, pubname, puballtables, pubinsert,\n"
- " pubupdate, pubdelete\n"
+ " pubupdate, pubdelete, ");
+ printACLColumn(&buf, "pubacl");
+ appendPQExpBuffer(&buf,
"FROM pg_catalog.pg_publication\n");
processSQLNamePattern(pset.db, &buf, pattern, false, false,
@@ -4985,7 +4987,7 @@ describePublications(const char *pattern)
for (i = 0; i < PQntuples(res); i++)
{
const char align = 'l';
- int ncols = 3;
+ int ncols = 4;
int nrows = 1;
int tables = 0;
PGresult *tabres;
@@ -5004,10 +5006,12 @@ describePublications(const char *pattern)
printTableAddHeader(&cont, gettext_noop("Inserts"), true, align);
printTableAddHeader(&cont, gettext_noop("Updates"), true, align);
printTableAddHeader(&cont, gettext_noop("Deletes"), true, align);
+ printTableAddHeader(&cont, gettext_noop("Access privileges"), true, align);
printTableAddCell(&cont, PQgetvalue(res, i, 3), false, false);
printTableAddCell(&cont, PQgetvalue(res, i, 4), false, false);
printTableAddCell(&cont, PQgetvalue(res, i, 5), false, false);
+ printTableAddCell(&cont, PQgetvalue(res, i, 6), false, false);
if (puballtables)
printfPQExpBuffer(&buf,
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 59519f068a..1cb06eeee2 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2705,6 +2705,7 @@ psql_completion(const char *text, int start, int end)
" UNION SELECT 'FUNCTION'"
" UNION SELECT 'LANGUAGE'"
" UNION SELECT 'LARGE OBJECT'"
+ " UNION SELECT 'PUBLICATION'"
" UNION SELECT 'SCHEMA'"
" UNION SELECT 'SEQUENCE'"
" UNION SELECT 'TABLE'"
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 5f42bde136..42cc94f47c 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 201702101
+#define CATALOG_VERSION_NO 201702135
#endif
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index bb7053a942..a1922e03d6 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3578,6 +3578,19 @@ DESCR("current user privilege on language by language name");
DATA(insert OID = 2267 ( has_language_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 2 0 16 "26 25" _null_ _null_ _null_ _null_ _null_ has_language_privilege_id _null_ _null_ _null_ ));
DESCR("current user privilege on language by language oid");
+DATA(insert OID = 4001 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "19 25 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_name_name _null_ _null_ _null_ ));
+DESCR("user privilege on publication by username, publication name");
+DATA(insert OID = 4002 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "19 26 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_name_id _null_ _null_ _null_ ));
+DESCR("user privilege on publication by username, publication oid");
+DATA(insert OID = 4003 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "26 25 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_id_name _null_ _null_ _null_ ));
+DESCR("user privilege on publication by user oid, publication name");
+DATA(insert OID = 4004 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "26 26 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_id_id _null_ _null_ _null_ ));
+DESCR("user privilege on publication by user oid, publication oid");
+DATA(insert OID = 4005 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 2 0 16 "25 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_name _null_ _null_ _null_ ));
+DESCR("current user privilege on publication by publication name");
+DATA(insert OID = 4006 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 2 0 16 "26 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_id _null_ _null_ _null_ ));
+DESCR("current user privilege on publication by publication oid");
+
DATA(insert OID = 2268 ( has_schema_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "19 25 25" _null_ _null_ _null_ _null_ _null_ has_schema_privilege_name_name _null_ _null_ _null_ ));
DESCR("user privilege on schema by username, schema name");
DATA(insert OID = 2269 ( has_schema_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "19 26 25" _null_ _null_ _null_ _null_ _null_ has_schema_privilege_name_id _null_ _null_ _null_ ));
diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h
index f3c4f3932b..bce81c373f 100644
--- a/src/include/catalog/pg_publication.h
+++ b/src/include/catalog/pg_publication.h
@@ -49,6 +49,9 @@ CATALOG(pg_publication,6104)
/* true if deletes are published */
bool pubdelete;
+#ifdef CATALOG_VARLEN /* variable-length fields start here */
+ aclitem pubacl[1]; /* access permissions */
+#endif
} FormData_pg_publication;
/* ----------------
@@ -63,13 +66,14 @@ typedef FormData_pg_publication *Form_pg_publication;
* ----------------
*/
-#define Natts_pg_publication 6
+#define Natts_pg_publication 7
#define Anum_pg_publication_pubname 1
#define Anum_pg_publication_pubowner 2
#define Anum_pg_publication_puballtables 3
#define Anum_pg_publication_pubinsert 4
#define Anum_pg_publication_pubupdate 5
#define Anum_pg_publication_pubdelete 6
+#define Anum_pg_publication_pubacl 7
typedef struct PublicationActions
{
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e0e94dd06b..840e443009 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1744,6 +1744,7 @@ typedef enum GrantObjectType
ACL_OBJECT_LANGUAGE, /* procedural language */
ACL_OBJECT_LARGEOBJECT, /* largeobject */
ACL_OBJECT_NAMESPACE, /* namespace */
+ ACL_OBJECT_PUBLICATION, /* publication */
ACL_OBJECT_TABLESPACE, /* tablespace */
ACL_OBJECT_TYPE /* type */
} GrantObjectType;
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index a34d8126b7..6d91656a37 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -159,6 +159,7 @@ typedef ArrayType Acl;
#define ACL_ALL_RIGHTS_NAMESPACE (ACL_USAGE|ACL_CREATE)
#define ACL_ALL_RIGHTS_TABLESPACE (ACL_CREATE)
#define ACL_ALL_RIGHTS_TYPE (ACL_USAGE)
+#define ACL_ALL_RIGHTS_PUBLICATION (ACL_USAGE)
/* operation codes for pg_*_aclmask */
typedef enum
@@ -274,6 +275,8 @@ extern AclMode pg_foreign_data_wrapper_aclmask(Oid fdw_oid, Oid roleid,
AclMode mask, AclMaskHow how);
extern AclMode pg_foreign_server_aclmask(Oid srv_oid, Oid roleid,
AclMode mask, AclMaskHow how);
+extern AclMode pg_publication_aclmask(Oid pub_oid, Oid roleid,
+ AclMode mask, AclMaskHow how);
extern AclMode pg_type_aclmask(Oid type_oid, Oid roleid,
AclMode mask, AclMaskHow how);
@@ -291,6 +294,7 @@ extern AclResult pg_namespace_aclcheck(Oid nsp_oid, Oid roleid, AclMode mode);
extern AclResult pg_tablespace_aclcheck(Oid spc_oid, Oid roleid, AclMode mode);
extern AclResult pg_foreign_data_wrapper_aclcheck(Oid fdw_oid, Oid roleid, AclMode mode);
extern AclResult pg_foreign_server_aclcheck(Oid srv_oid, Oid roleid, AclMode mode);
+extern AclResult pg_publication_aclcheck(Oid pub_oid, Oid roleid, AclMode mode);
extern AclResult pg_type_aclcheck(Oid type_oid, Oid roleid, AclMode mode);
extern void aclcheck_error(AclResult aclerr, AclObjectKind objectkind,
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index e63612e0d5..f82c539cf9 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -75,10 +75,10 @@ ERROR: relation "testpub_tbl1" is already member of publication "testpub_fortbl
CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1;
ERROR: publication "testpub_fortbl" already exists
\dRp+ testpub_fortbl
- Publication testpub_fortbl
- Inserts | Updates | Deletes
----------+---------+---------
- t | t | t
+ Publication testpub_fortbl
+ Inserts | Updates | Deletes | Access privileges
+---------+---------+---------+-------------------
+ t | t | t |
Tables:
"pub_test.testpub_nopk"
"public.testpub_tbl1"
@@ -116,10 +116,10 @@ Publications:
"testpub_fortbl"
\dRp+ testpub_default
- Publication testpub_default
- Inserts | Updates | Deletes
----------+---------+---------
- t | t | t
+ Publication testpub_default
+ Inserts | Updates | Deletes | Access privileges
+---------+---------+---------+-------------------
+ t | t | t |
Tables:
"pub_test.testpub_nopk"
"public.testpub_tbl1"
@@ -160,18 +160,20 @@ REVOKE CREATE ON DATABASE regression FROM regress_publication_user2;
DROP VIEW testpub_view;
DROP TABLE testpub_tbl1;
\dRp+ testpub_default
- Publication testpub_default
- Inserts | Updates | Deletes
----------+---------+---------
- t | t | t
+ Publication testpub_default
+ Inserts | Updates | Deletes | Access privileges
+---------+---------+---------+-------------------
+ t | t | t |
(1 row)
ALTER PUBLICATION testpub_default OWNER TO regress_publication_user2;
+GRANT USAGE ON PUBLICATION testpub_default TO regress_publication_user;
\dRp+ testpub_default
- Publication testpub_default
- Inserts | Updates | Deletes
----------+---------+---------
- t | t | t
+ Publication testpub_default
+ Inserts | Updates | Deletes | Access privileges
+---------+---------+---------+-------------------------------------------------------
+ t | t | t | regress_publication_user2=U/regress_publication_user2+
+ | | | regress_publication_user=U/regress_publication_user2
(1 row)
DROP PUBLICATION testpub_default;
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 7b322bb7d9..6f83f66207 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -97,6 +97,8 @@ CREATE PUBLICATION testpub2; -- ok
ALTER PUBLICATION testpub_default OWNER TO regress_publication_user2;
+GRANT USAGE ON PUBLICATION testpub_default TO regress_publication_user;
+
\dRp+ testpub_default
DROP PUBLICATION testpub_default;
diff --git a/src/test/subscription/t/003_privileges.pl b/src/test/subscription/t/003_privileges.pl
new file mode 100644
index 0000000000..86fd866e28
--- /dev/null
+++ b/src/test/subscription/t/003_privileges.pl
@@ -0,0 +1,72 @@
+# Tests of privileges for logical replication
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More tests => 2;
+
+my $node_publisher = get_new_node('publisher');
+$node_publisher->init(allows_streaming => 'logical');
+$node_publisher->start;
+
+my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+
+my $node_subscriber = get_new_node('subscriber');
+$node_subscriber->init(allows_streaming => 'logical');
+$node_subscriber->start;
+
+$node_publisher->safe_psql('postgres', qq(
+CREATE USER test_user1;
+CREATE USER test_user2 REPLICATION;
+GRANT CREATE ON DATABASE postgres TO test_user1;
+
+SET ROLE test_user1;
+CREATE TABLE test1 (a int PRIMARY KEY, b text);
+CREATE PUBLICATION mypub1 FOR TABLE test1;
+));
+
+my $appname = 'tap_sub';
+
+$node_subscriber->safe_psql('postgres', qq(
+CREATE TABLE test1 (a int PRIMARY KEY, b text);
+CREATE SUBSCRIPTION mysub1 CONNECTION '$publisher_connstr user=test_user2 application_name=$appname' PUBLICATION mypub1;
+));
+
+$node_publisher->safe_psql('postgres', qq(
+SET ROLE test_user1;
+INSERT INTO test1 VALUES (1, 'one');
+));
+
+my $log = TestLib::slurp_file($node_publisher->logfile);
+like($log, qr/permission denied for publication mypub1/, "permission denied on publication");
+
+$node_publisher->safe_psql('postgres', qq(
+SET ROLE test_user1;
+GRANT USAGE ON PUBLICATION mypub1 TO test_user2;
+));
+
+# drop and recreate subscription so it sees the newly granted
+# privileges
+$node_subscriber->safe_psql('postgres', qq(
+DROP SUBSCRIPTION mysub1;
+CREATE SUBSCRIPTION mysub1 CONNECTION '$publisher_connstr user=test_user2 application_name=$appname' PUBLICATION mypub1;
+));
+
+$node_publisher->safe_psql('postgres', qq(
+SET ROLE test_user1;
+INSERT INTO test1 VALUES (2, 'two');
+));
+
+my $caughtup_query =
+ "SELECT pg_current_wal_location() <= replay_location FROM pg_stat_replication WHERE application_name = '$appname';";
+$node_publisher->poll_query_until('postgres', $caughtup_query)
+ or die "Timed out while waiting for subscriber to catch up";
+
+my $result = $node_subscriber->safe_psql('postgres', qq(
+SELECT a, b FROM test1;
+));
+
+is($result, '2|two', 'replication catches up after privileges granted');
+
+$node_subscriber->stop;
+$node_publisher->stop;
--
2.11.1
0004-Add-CREATE-SUBSCRIPTION-privilege-on-databases.patchtext/x-patch; name=0004-Add-CREATE-SUBSCRIPTION-privilege-on-databases.patchDownload
From 89e8b74439f2834b76c5b8939f4c42b274036c40 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 13 Feb 2017 15:42:29 -0500
Subject: [PATCH 4/6] Add CREATE SUBSCRIPTION privilege on databases
This new privilege allows the creation of subscriptions. This was
previously only allowed for superusers.
---
doc/src/sgml/logical-replication.sgml | 3 ++-
doc/src/sgml/ref/create_subscription.sgml | 6 +++++
doc/src/sgml/ref/grant.sgml | 12 +++++++++-
src/backend/catalog/aclchk.c | 4 ++++
src/backend/commands/subscriptioncmds.c | 38 +++++++++++++++++++-----------
src/backend/parser/gram.y | 6 +++++
src/backend/utils/adt/acl.c | 9 +++++++
src/bin/pg_dump/dumputils.c | 2 ++
src/include/nodes/parsenodes.h | 3 ++-
src/include/utils/acl.h | 5 ++--
src/test/regress/expected/subscription.out | 14 ++++++++++-
src/test/regress/sql/subscription.sql | 14 ++++++++++-
12 files changed, 95 insertions(+), 21 deletions(-)
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index b6636d76b1..98411af0e6 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -315,7 +315,8 @@ <title>Security</title>
</para>
<para>
- To create a subscription, the user must be a superuser.
+ To create a subscription, the user must have the <literal>CREATE
+ SUBSCRIPTION</literal> privilege in the database.
</para>
<para>
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 59e5ad00c8..830c612fad 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -131,6 +131,12 @@ <title>Parameters</title>
<title>Notes</title>
<para>
+ To create a subscription, the invoking user must have the
+ <literal>CREATE SUBSCRIPTION</> privilege for the current database. (Of
+ course, superusers bypass this check.)
+ </para>
+
+ <para>
See <xref linkend="streaming-replication-authentication"> for details on
how to configure access control between the subscription and the
publication instance.
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index 996d183561..f4f9afe7da 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -38,7 +38,7 @@
| ALL SEQUENCES IN SCHEMA <replaceable class="PARAMETER">schema_name</replaceable> [, ...] }
TO <replaceable class="PARAMETER">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
-GRANT { { CREATE | CONNECT | TEMPORARY | TEMP } [, ...] | ALL [ PRIVILEGES ] }
+GRANT { { CREATE | CONNECT | TEMPORARY | TEMP | CREATE SUBSCRIPTION } [, ...] | ALL [ PRIVILEGES ] }
ON DATABASE <replaceable>database_name</replaceable> [, ...]
TO <replaceable class="PARAMETER">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
@@ -310,6 +310,15 @@ <title>GRANT on Database Objects</title>
</varlistentry>
<varlistentry>
+ <term>CREATE SUBSCRIPTION</term>
+ <listitem>
+ <para>
+ For databases, allows new subscriptions to be created within the database.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term>CONNECT</term>
<listitem>
<para>
@@ -553,6 +562,7 @@ <title>Notes</title>
X -- EXECUTE
U -- USAGE
C -- CREATE
+ S -- CREATE SUBSCRIPTION
c -- CONNECT
T -- TEMPORARY
arwdDxtp -- ALL PRIVILEGES (for tables, varies for other objects)
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 20ab6d5b0d..f38e4802bc 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -3365,6 +3365,8 @@ string_to_privilege(const char *privname)
return ACL_CONNECT;
if (strcmp(privname, "publication") == 0)
return ACL_PUBLICATION;
+ if (strcmp(privname, "create subscription") == 0)
+ return ACL_CREATE_SUBSCRIPTION;
if (strcmp(privname, "rule") == 0)
return 0; /* ignore old RULE privileges */
ereport(ERROR,
@@ -3404,6 +3406,8 @@ privilege_to_string(AclMode privilege)
return "CONNECT";
case ACL_PUBLICATION:
return "PUBLICATION";
+ case ACL_CREATE_SUBSCRIPTION:
+ return "CREATE SUBSCRIPTION";
default:
elog(ERROR, "unrecognized privilege: %d", (int) privilege);
}
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index ab21e64b48..ed2a294c64 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -25,6 +25,7 @@
#include "catalog/pg_type.h"
#include "catalog/pg_subscription.h"
+#include "commands/dbcommands.h"
#include "commands/defrem.h"
#include "commands/event_trigger.h"
#include "commands/subscriptioncmds.h"
@@ -220,11 +221,13 @@ CreateSubscription(CreateSubscriptionStmt *stmt)
char originname[NAMEDATALEN];
bool create_slot;
List *publications;
+ AclResult aclresult;
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to create subscriptions"))));
+ /* must have CREATE SUBSCRIPTION privilege on database */
+ aclresult = pg_database_aclcheck(MyDatabaseId, GetUserId(), ACL_CREATE_SUBSCRIPTION);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_DATABASE,
+ get_database_name(MyDatabaseId));
rel = heap_open(SubscriptionRelationId, RowExclusiveLock);
@@ -575,17 +578,24 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
if (form->subowner == newOwnerId)
return;
- if (!pg_subscription_ownercheck(HeapTupleGetOid(tup), GetUserId()))
- aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_SUBSCRIPTION,
- NameStr(form->subname));
+ 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 subscription \"%s\"",
- NameStr(form->subname)),
- errhint("The owner of an subscription must be a superuser.")));
+ /* Must be owner */
+ if (!pg_subscription_ownercheck(HeapTupleGetOid(tup), GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_SUBSCRIPTION,
+ NameStr(form->subname));
+
+ /* Must be able to become new owner */
+ check_is_member_of_role(GetUserId(), newOwnerId);
+
+ /* New owner must have CREATE SUBSCRIPTION privilege on database */
+ aclresult = pg_database_aclcheck(MyDatabaseId, newOwnerId, ACL_CREATE_SUBSCRIPTION);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_DATABASE,
+ get_database_name(MyDatabaseId));
+ }
form->subowner = newOwnerId;
CatalogTupleUpdate(rel, &tup->t_self, tup);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 2f05264e84..55a5d0a7c7 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -6777,6 +6777,12 @@ privilege: SELECT opt_column_list
n->cols = $2;
$$ = n;
}
+ | CREATE ColId
+ {
+ AccessPriv *n = makeNode(AccessPriv);
+ n->priv_name = psprintf("create %s", $2);
+ $$ = n;
+ }
;
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index e84c8d80c7..7f4513117a 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -320,6 +320,9 @@ aclparse(const char *s, AclItem *aip)
case ACL_PUBLICATION_CHR:
read = ACL_PUBLICATION;
break;
+ case ACL_CREATE_SUBSCRIPTION_CHR:
+ read = ACL_CREATE_SUBSCRIPTION;
+ break;
case 'R': /* ignore old RULE privileges */
read = 0;
break;
@@ -1640,6 +1643,8 @@ convert_priv_string(text *priv_type_text)
return ACL_CONNECT;
if (pg_strcasecmp(priv_type, "PUBLICATION") == 0)
return ACL_PUBLICATION;
+ if (pg_strcasecmp(priv_type, "CREATE SUBSCRIPTION") == 0)
+ return ACL_CREATE_SUBSCRIPTION;
if (pg_strcasecmp(priv_type, "RULE") == 0)
return 0; /* ignore old RULE privileges */
@@ -1738,6 +1743,8 @@ convert_aclright_to_string(int aclright)
return "CONNECT";
case ACL_PUBLICATION:
return "PUBLICATION";
+ case ACL_CREATE_SUBSCRIPTION:
+ return "CREATE SUBSCRIPTION";
default:
elog(ERROR, "unrecognized aclright: %d", aclright);
return NULL;
@@ -3075,6 +3082,8 @@ convert_database_priv_string(text *priv_type_text)
{"TEMP WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_CREATE_TEMP)},
{"CONNECT", ACL_CONNECT},
{"CONNECT WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_CONNECT)},
+ {"CREATE SUBSCRIPTION", ACL_CREATE_SUBSCRIPTION},
+ {"CREATE SUBSCRIPTION WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_CREATE_SUBSCRIPTION)},
{NULL, 0}
};
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 8b6d70f2cb..00c6b1b411 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -534,6 +534,8 @@ do { \
CONVERT_PRIV('C', "CREATE");
CONVERT_PRIV('c', "CONNECT");
CONVERT_PRIV('T', "TEMPORARY");
+ if (remoteVersion >= 100000)
+ CONVERT_PRIV('S', "CREATE SUBSCRIPTION");
}
else if (strcmp(type, "TABLESPACE") == 0)
CONVERT_PRIV('C', "CREATE");
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 840e443009..488aec08ce 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -76,7 +76,8 @@ typedef uint32 AclMode; /* a bitmask of privilege bits */
#define ACL_CREATE_TEMP (1<<10) /* for databases */
#define ACL_CONNECT (1<<11) /* for databases */
#define ACL_PUBLICATION (1<<12)
-#define N_ACL_RIGHTS 13 /* 1 plus the last 1<<x */
+#define ACL_CREATE_SUBSCRIPTION (1<<13)
+#define N_ACL_RIGHTS 14 /* 1 plus the last 1<<x */
#define ACL_NO_RIGHTS 0
/* Currently, SELECT ... FOR [KEY] UPDATE/SHARE requires UPDATE privileges */
#define ACL_SELECT_FOR_UPDATE ACL_UPDATE
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 6d91656a37..fdd12767f0 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -140,9 +140,10 @@ typedef ArrayType Acl;
#define ACL_CREATE_TEMP_CHR 'T'
#define ACL_CONNECT_CHR 'c'
#define ACL_PUBLICATION_CHR 'p'
+#define ACL_CREATE_SUBSCRIPTION_CHR 'S'
/* string holding all privilege code chars, in order by bitmask position */
-#define ACL_ALL_RIGHTS_STR "arwdDxtXUCTcp"
+#define ACL_ALL_RIGHTS_STR "arwdDxtXUCTcpS"
/*
* Bitmasks defining "all rights" for each supported object type
@@ -150,7 +151,7 @@ typedef ArrayType Acl;
#define ACL_ALL_RIGHTS_COLUMN (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_REFERENCES)
#define ACL_ALL_RIGHTS_RELATION (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_TRUNCATE|ACL_REFERENCES|ACL_TRIGGER|ACL_PUBLICATION)
#define ACL_ALL_RIGHTS_SEQUENCE (ACL_USAGE|ACL_SELECT|ACL_UPDATE)
-#define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP|ACL_CONNECT)
+#define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP|ACL_CONNECT|ACL_CREATE_SUBSCRIPTION)
#define ACL_ALL_RIGHTS_FDW (ACL_USAGE)
#define ACL_ALL_RIGHTS_FOREIGN_SERVER (ACL_USAGE)
#define ACL_ALL_RIGHTS_FUNCTION (ACL_EXECUTE)
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 2ccec98b15..7677fbb79f 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -2,7 +2,8 @@
-- SUBSCRIPTION
--
CREATE ROLE regress_subscription_user LOGIN SUPERUSER;
-SET SESSION AUTHORIZATION 'regress_subscription_user';
+CREATE ROLE regress_subscription_user2 LOGIN;
+SET SESSION AUTHORIZATION regress_subscription_user;
-- fail - no publications
CREATE SUBSCRIPTION testsub CONNECTION 'foo';
ERROR: syntax error at or near ";"
@@ -62,5 +63,16 @@ ALTER SUBSCRIPTION testsub DISABLE;
COMMIT;
DROP SUBSCRIPTION testsub NODROP SLOT;
+-- permissions
+set client_min_messages to error;
+SET SESSION AUTHORIZATION regress_subscription_user2;
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (DISABLED, NOCREATE SLOT); -- fail
+ERROR: permission denied for database regression
+SET SESSION AUTHORIZATION regress_subscription_user;
+GRANT CREATE SUBSCRIPTION ON DATABASE regression TO regress_subscription_user2;
+SET SESSION AUTHORIZATION regress_subscription_user2;
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (DISABLED, NOCREATE SLOT); -- ok
+reset client_min_messages;
+DROP SUBSCRIPTION testsub2 NODROP SLOT;
RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 68c17d5cfd..30ad03e887 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -3,7 +3,8 @@
--
CREATE ROLE regress_subscription_user LOGIN SUPERUSER;
-SET SESSION AUTHORIZATION 'regress_subscription_user';
+CREATE ROLE regress_subscription_user2 LOGIN;
+SET SESSION AUTHORIZATION regress_subscription_user;
-- fail - no publications
CREATE SUBSCRIPTION testsub CONNECTION 'foo';
@@ -40,5 +41,16 @@ CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub
DROP SUBSCRIPTION testsub NODROP SLOT;
+-- permissions
+set client_min_messages to error;
+SET SESSION AUTHORIZATION regress_subscription_user2;
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (DISABLED, NOCREATE SLOT); -- fail
+SET SESSION AUTHORIZATION regress_subscription_user;
+GRANT CREATE SUBSCRIPTION ON DATABASE regression TO regress_subscription_user2;
+SET SESSION AUTHORIZATION regress_subscription_user2;
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (DISABLED, NOCREATE SLOT); -- ok
+reset client_min_messages;
+DROP SUBSCRIPTION testsub2 NODROP SLOT;
+
RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
--
2.11.1
0005-Add-subscription-apply-worker-privilege-checks.patchtext/x-patch; name=0005-Add-subscription-apply-worker-privilege-checks.patchDownload
From 4766aa28c9f95d2c65eaeb25303b410d6d76e5b4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 13 Feb 2017 16:28:36 -0500
Subject: [PATCH 5/6] Add subscription apply worker privilege checks
The subscription apply worker now checks INSERT/UPDATE/DELETE privileges
on a table before writing to it. This was previously not checked, but
was not necessary since a subscription owner had to be a superuser. But
we now allow other users to own subscriptions.
---
doc/src/sgml/logical-replication.sgml | 4 +++-
src/backend/replication/logical/relation.c | 24 ++++++++++++++++++++++++
src/backend/replication/logical/worker.c | 3 +++
src/include/replication/logicalrelation.h | 5 +++++
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 98411af0e6..262008ee3b 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -326,7 +326,9 @@ <title>Security</title>
<para>
The subscription apply process will run in the local database with the
- privileges of a superuser.
+ privileges of the subscription owner. The subscription owner, if it is not
+ a superuser, therefore needs to have appropriate privileges on the tables
+ the subscription is operating on.
</para>
<para>
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index d8dc0c7194..4faee8323c 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -19,6 +19,7 @@
#include "access/heapam.h"
#include "access/sysattr.h"
#include "catalog/namespace.h"
+#include "miscadmin.h"
#include "nodes/makefuncs.h"
#include "replication/logicalrelation.h"
#include "replication/worker_internal.h"
@@ -78,6 +79,18 @@ logicalrep_relmap_invalidate_cb(Datum arg, Oid reloid)
}
/*
+ * Syscache invalidation callback for our relation map cache.
+ *
+ * Same as the relcache callback, except that we just invalidate all cache
+ * entries all the time.
+ */
+static void
+logicalrep_relmap_syscache_invalidate_cb(Datum arg, int cacheid, uint32 hashvalue)
+{
+ logicalrep_relmap_invalidate_cb(arg, InvalidOid);
+}
+
+/*
* Initialize the relation map cache.
*/
static void
@@ -113,6 +126,8 @@ logicalrep_relmap_init()
/* Watch for invalidation events. */
CacheRegisterRelcacheCallback(logicalrep_relmap_invalidate_cb,
(Datum) 0);
+ CacheRegisterSyscacheCallback(AUTHOID, logicalrep_relmap_syscache_invalidate_cb,
+ (Datum) 0);
CacheRegisterSyscacheCallback(TYPEOID, logicalrep_typmap_invalidate_cb,
(Datum) 0);
}
@@ -277,6 +292,15 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
remoterel->nspname, remoterel->relname)));
/*
+ * Cache ACL results. If they are changed while the worker is active,
+ * the relcache and syscache invalidation will ensure that we get here
+ * again to recompute this.
+ */
+ entry->insert_aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_INSERT);
+ entry->update_aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_UPDATE);
+ entry->delete_aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_DELETE);
+
+ /*
* Build the mapping of local attribute numbers to remote attribute
* numbers and validate that we don't miss any replicated columns
* as that would result in potentially unwanted data loss.
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 0b19feca40..69c4770bea 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -515,6 +515,7 @@ apply_handle_insert(StringInfo s)
relid = logicalrep_read_insert(s, &newtup);
rel = logicalrep_rel_open(relid, RowExclusiveLock);
+ aclcheck_error(rel->insert_aclresult, ACL_KIND_CLASS, get_rel_name(rel->localreloid));
/* Initialize the executor state. */
estate = create_estate_for_relation(rel);
@@ -603,6 +604,7 @@ apply_handle_update(StringInfo s)
relid = logicalrep_read_update(s, &has_oldtup, &oldtup,
&newtup);
rel = logicalrep_rel_open(relid, RowExclusiveLock);
+ aclcheck_error(rel->update_aclresult, ACL_KIND_CLASS, get_rel_name(rel->localreloid));
/* Check if we can do the update. */
check_relation_updatable(rel);
@@ -708,6 +710,7 @@ apply_handle_delete(StringInfo s)
relid = logicalrep_read_delete(s, &oldtup);
rel = logicalrep_rel_open(relid, RowExclusiveLock);
+ aclcheck_error(rel->delete_aclresult, ACL_KIND_CLASS, get_rel_name(rel->localreloid));
/* Check if we can do the delete. */
check_relation_updatable(rel);
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index 7fb7fbfb4d..f9d5144f61 100644
--- a/src/include/replication/logicalrelation.h
+++ b/src/include/replication/logicalrelation.h
@@ -25,6 +25,11 @@ typedef struct LogicalRepRelMapEntry
* remote ones */
bool updatable; /* Can apply updates/deletes? */
+ /* Cache of ACL results */
+ AclResult insert_aclresult;
+ AclResult update_aclresult;
+ AclResult delete_aclresult;
+
/* Sync state. */
char state;
XLogRecPtr statelsn;
--
2.11.1
0006-Change-logical-replication-pg_hba.conf-use.patchtext/x-patch; name=0006-Change-logical-replication-pg_hba.conf-use.patchDownload
From 903b7e82924eff3dcfa225ffd236926b9a356ceb Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 13 Feb 2017 16:50:29 -0500
Subject: [PATCH 6/6] Change logical replication pg_hba.conf use
Logical replication no longer uses the "replication" keyword. It just
matches database entries in the normal way. The "replication" keyword
now only applies to physical replication.
---
doc/src/sgml/client-auth.sgml | 2 +-
doc/src/sgml/logical-replication.sgml | 8 +++-----
src/backend/libpq/hba.c | 4 ++--
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 231fc40fc3..4306fb3ea1 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -193,7 +193,7 @@ <title>The <filename>pg_hba.conf</filename> File</title>
members of the role, directly or indirectly, and not just by
virtue of being a superuser.
The value <literal>replication</> specifies that the record
- matches if a replication connection is requested (note that
+ matches if a physical replication connection is requested (note that
replication connections do not specify any particular database).
Otherwise, this is the name of
a specific <productname>PostgreSQL</productname> database.
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 262008ee3b..9631bda11f 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -295,11 +295,9 @@ <title>Monitoring</title>
<title>Security</title>
<para>
- Logical replication connections occur in the same way as with physical streaming
- replication. It requires access to be explicitly given using
- <filename>pg_hba.conf</filename>. The role used for the replication
- connection must have the <literal>REPLICATION</literal> attribute. This
- gives a role access to both logical and physical replication.
+ The role used for the replication connection must have
+ the <literal>REPLICATION</literal> attribute. Access for the role must be
+ configured in <filename>pg_hba.conf</filename>.
</para>
<para>
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index be63a4bc63..0aeecdd003 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -614,9 +614,9 @@ check_db(const char *dbname, const char *role, Oid roleid, List *tokens)
foreach(cell, tokens)
{
tok = lfirst(cell);
- if (am_walsender)
+ if (am_walsender && !am_db_walsender)
{
- /* walsender connections can only match replication keyword */
+ /* physical replication walsender connections can only match replication keyword */
if (token_is_keyword(tok, "replication"))
return true;
}
--
2.11.1
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
0002 Add PUBLICATION privilege
Add a new privilege kind to tables to determine whether they can be
added to a publication.
I'm not convinced that it really makes sense to have PUBLICATION of a
table be independent from the rights an owner of a table has. We don't
allow other ALTER commands on objects based on GRANT'able rights, in
general, so I'm not really sure that it makes sense to do so here.
The downside of adding these privileges is that we're burning through
the last few bits in the ACLMASK for a privilege that doesn't really
seem like it's something that would be GRANT'd in general usage.
I have similar reservations regarding the proposed SUBSCRIPTION
privilege.
I'm certainly all for removing the need for users to be the superuser
for such commands, just not sure that they should be GRANT'able
privileges instead of privileges which the owner of the relation or
database has.
Thanks!
Stephen
On 2/18/17 18:06, Stephen Frost wrote:
I'm not convinced that it really makes sense to have PUBLICATION of a
table be independent from the rights an owner of a table has. We don't
allow other ALTER commands on objects based on GRANT'able rights, in
general, so I'm not really sure that it makes sense to do so here.
The REFERENCES and TRIGGER privileges are very similar in principle.
The downside of adding these privileges is that we're burning through
the last few bits in the ACLMASK for a privilege that doesn't really
seem like it's something that would be GRANT'd in general usage.
I don't see any reason why we couldn't increase the size of AclMode if
it becomes necessary.
I'm certainly all for removing the need for users to be the superuser
for such commands, just not sure that they should be GRANT'able
privileges instead of privileges which the owner of the relation or
database has.
Then you couldn't set up a replication structure involving tables owned
by different users without resorting to blunt instruments like having
everything owned by the same user or using superusers.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 2/18/17 18:06, Stephen Frost wrote:
I'm not convinced that it really makes sense to have PUBLICATION of a
table be independent from the rights an owner of a table has. We don't
allow other ALTER commands on objects based on GRANT'able rights, in
general, so I'm not really sure that it makes sense to do so here.The REFERENCES and TRIGGER privileges are very similar in principle.
TRIGGER is a great example of an, ultimately, poorly chosen privilege to
GRANT away, with a good history of discussion about it:
/messages/by-id/21827.1166115978@sss.pgh.pa.us
/messages/by-id/7389.1405554356@sss.pgh.pa.us
Further, how would RLS be handled with publication? I've been assuming
that it's simply ignored, but that's clearly wrong if a non-owner can
publish a table that they just have SELECT,PUBLISH on, isn't it?
The downside of adding these privileges is that we're burning through
the last few bits in the ACLMASK for a privilege that doesn't really
seem like it's something that would be GRANT'd in general usage.I don't see any reason why we couldn't increase the size of AclMode if
it becomes necessary.
I've fought exactly that argument before and there is a good deal of
entirely reasonable push-back on increasing our catalogs by so much.
I'm certainly all for removing the need for users to be the superuser
for such commands, just not sure that they should be GRANT'able
privileges instead of privileges which the owner of the relation or
database has.Then you couldn't set up a replication structure involving tables owned
by different users without resorting to blunt instruments like having
everything owned by the same user or using superusers.
That's not correct- you would simply need a user who is considered an
owner for all of the objects which you want to replicate, that doesn't
have to be a *direct* owner or a superuser.
The tables can have individual roles, with some admin role who is a
member of those other roles, or another mechanism (as Simon has proposed
not too long ago...) to have a given role be considered equivilant to an
owner for all of the relations in a particular database.
Thanks!
Stephen
On 28/02/17 04:10, Stephen Frost wrote:
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 2/18/17 18:06, Stephen Frost wrote:
I'm not convinced that it really makes sense to have PUBLICATION of a
table be independent from the rights an owner of a table has. We don't
allow other ALTER commands on objects based on GRANT'able rights, in
general, so I'm not really sure that it makes sense to do so here.The REFERENCES and TRIGGER privileges are very similar in principle.
TRIGGER is a great example of an, ultimately, poorly chosen privilege to
GRANT away, with a good history of discussion about it:/messages/by-id/21827.1166115978@sss.pgh.pa.us
/messages/by-id/7389.1405554356@sss.pgh.pa.usFurther, how would RLS be handled with publication? I've been assuming
that it's simply ignored, but that's clearly wrong if a non-owner can
publish a table that they just have SELECT,PUBLISH on, isn't it?
I don't see why would PUBLISH care about RLS.
The downside of adding these privileges is that we're burning through
the last few bits in the ACLMASK for a privilege that doesn't really
seem like it's something that would be GRANT'd in general usage.I don't see any reason why we couldn't increase the size of AclMode if
it becomes necessary.I've fought exactly that argument before and there is a good deal of
entirely reasonable push-back on increasing our catalogs by so much.
Hmm to be honest I don't see what's the issue there.
I'm certainly all for removing the need for users to be the superuser
for such commands, just not sure that they should be GRANT'able
privileges instead of privileges which the owner of the relation or
database has.Then you couldn't set up a replication structure involving tables owned
by different users without resorting to blunt instruments like having
everything owned by the same user or using superusers.That's not correct- you would simply need a user who is considered an
owner for all of the objects which you want to replicate, that doesn't
have to be a *direct* owner or a superuser.The tables can have individual roles, with some admin role who is a
member of those other roles, or another mechanism (as Simon has proposed
not too long ago...) to have a given role be considered equivilant to an
owner for all of the relations in a particular database.
I do agree with this though. And I also agree with the overall sentiment
that we don't need special PUBLICATION privilege on tables.
I do like the rest of the patch.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Petr Jelinek (petr.jelinek@2ndquadrant.com) wrote:
On 28/02/17 04:10, Stephen Frost wrote:
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 2/18/17 18:06, Stephen Frost wrote:
I'm not convinced that it really makes sense to have PUBLICATION of a
table be independent from the rights an owner of a table has. We don't
allow other ALTER commands on objects based on GRANT'able rights, in
general, so I'm not really sure that it makes sense to do so here.The REFERENCES and TRIGGER privileges are very similar in principle.
TRIGGER is a great example of an, ultimately, poorly chosen privilege to
GRANT away, with a good history of discussion about it:/messages/by-id/21827.1166115978@sss.pgh.pa.us
/messages/by-id/7389.1405554356@sss.pgh.pa.usFurther, how would RLS be handled with publication? I've been assuming
that it's simply ignored, but that's clearly wrong if a non-owner can
publish a table that they just have SELECT,PUBLISH on, isn't it?I don't see why would PUBLISH care about RLS.
Perhaps I'm missing something here, in which case I would certainly
welcome some clarification, but in curreng PG I can GRANT you access to
a table and then limit what you can see with it using RLS and policies.
If I then GRANT you the PUBLISH right- shouldn't that also respect the
RLS setting and the policies set on the table? Otherwise, PUBLISH is
allowing you to gain access to the data in the table that you wouldn't
normally be able to see with a simple SELECT.
We don't really even need to get to the RLS level to consider this
situation- what about column-level privileges?
Will users really understand that the PUBLISH right actually allows
complete access to the entire relation, rather than just the ability for
a user to PUBLISH what they are currently about to SELECT? It certainly
doesn't seem intuitive to me, which is why I am concerned that it's
going to lead to confusion and bug/security reports down the road, or,
worse, poorly configured systems.
The downside of adding these privileges is that we're burning through
the last few bits in the ACLMASK for a privilege that doesn't really
seem like it's something that would be GRANT'd in general usage.I don't see any reason why we couldn't increase the size of AclMode if
it becomes necessary.I've fought exactly that argument before and there is a good deal of
entirely reasonable push-back on increasing our catalogs by so much.Hmm to be honest I don't see what's the issue there.
It's a not-insignificant increase in our system catalog data, plus a
bunch of work for whomever ends up being the one to want to push us
over. The other approach would be to make the ACL system understand
that not every privilege applies to every object, but that's would also
be a bunch of work (most likely more...).
Thanks!
Stephen
On 2/27/17 22:10, Stephen Frost wrote:
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 2/18/17 18:06, Stephen Frost wrote:
I'm not convinced that it really makes sense to have PUBLICATION of a
table be independent from the rights an owner of a table has. We don't
allow other ALTER commands on objects based on GRANT'able rights, in
general, so I'm not really sure that it makes sense to do so here.The REFERENCES and TRIGGER privileges are very similar in principle.
TRIGGER is a great example of an, ultimately, poorly chosen privilege to
GRANT away, with a good history of discussion about it:/messages/by-id/21827.1166115978@sss.pgh.pa.us
/messages/by-id/7389.1405554356@sss.pgh.pa.us
Those discussions about the trigger privileges are valid, but the actual
reason why this is a problem is because our trigger implementation is
broken by default. In the SQL standard, triggers are executed as the
table owner, so the trigger procedure does not have full account access
to the role that is causing the trigger to fire. This is an independent
problem that deserves consideration, but it does not invalidate the kind
of privilege that can be granted.
Then you couldn't set up a replication structure involving tables owned
by different users without resorting to blunt instruments like having
everything owned by the same user or using superusers.That's not correct- you would simply need a user who is considered an
owner for all of the objects which you want to replicate, that doesn't
have to be a *direct* owner or a superuser.The tables can have individual roles, with some admin role who is a
member of those other roles, or another mechanism (as Simon has proposed
not too long ago...) to have a given role be considered equivilant to an
owner for all of the relations in a particular database.
I'm not really following what you are saying here. It seems to me that
you are describing a new kind of facility that gives a role a given
capability with respect to a table.
If so, we already have that, namely privileges. If not, please elaborate.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/3/17 10:07, Stephen Frost wrote:
Will users really understand that the PUBLISH right actually allows
complete access to the entire relation, rather than just the ability for
a user to PUBLISH what they are currently about to SELECT? It certainly
doesn't seem intuitive to me, which is why I am concerned that it's
going to lead to confusion and bug/security reports down the road, or,
worse, poorly configured systems.
Well, this is a new system with its own properties, which is why I'm
proposing a new way to manage access. If it were just the same as the
existing stuff, we wouldn't need this conversation. I'm interested in
other ideas.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 10/03/17 20:02, Peter Eisentraut wrote:
On 2/27/17 22:10, Stephen Frost wrote:
Peter,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 2/18/17 18:06, Stephen Frost wrote:
I'm not convinced that it really makes sense to have PUBLICATION of a
table be independent from the rights an owner of a table has. We don't
allow other ALTER commands on objects based on GRANT'able rights, in
general, so I'm not really sure that it makes sense to do so here.The REFERENCES and TRIGGER privileges are very similar in principle.
TRIGGER is a great example of an, ultimately, poorly chosen privilege to
GRANT away, with a good history of discussion about it:/messages/by-id/21827.1166115978@sss.pgh.pa.us
/messages/by-id/7389.1405554356@sss.pgh.pa.usThose discussions about the trigger privileges are valid, but the actual
reason why this is a problem is because our trigger implementation is
broken by default. In the SQL standard, triggers are executed as the
table owner, so the trigger procedure does not have full account access
to the role that is causing the trigger to fire. This is an independent
problem that deserves consideration, but it does not invalidate the kind
of privilege that can be granted.Then you couldn't set up a replication structure involving tables owned
by different users without resorting to blunt instruments like having
everything owned by the same user or using superusers.That's not correct- you would simply need a user who is considered an
owner for all of the objects which you want to replicate, that doesn't
have to be a *direct* owner or a superuser.The tables can have individual roles, with some admin role who is a
member of those other roles, or another mechanism (as Simon has proposed
not too long ago...) to have a given role be considered equivilant to an
owner for all of the relations in a particular database.I'm not really following what you are saying here. It seems to me that
you are describing a new kind of facility that gives a role a given
capability with respect to a table.If so, we already have that, namely privileges. If not, please elaborate.
My understanding of what Shephen is proposing is, you have "ownerA" of
tableA and "ownerB" of tableB, then you want role "publishe"r to be able
to publish those, so you simply grant it the "ownerA" and "ownerB"
roles. Obviously that might is many situations mean that the "publisher"
role potentially also gets sweeping privileges to other tables which may
not be desirable.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Mar 14, 2017 at 2:41 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
My understanding of what Shephen is proposing is, you have "ownerA" of
tableA and "ownerB" of tableB, then you want role "publishe"r to be able
to publish those, so you simply grant it the "ownerA" and "ownerB"
roles. Obviously that might is many situations mean that the "publisher"
role potentially also gets sweeping privileges to other tables which may
not be desirable.
I didn't hear Stephen propose that "publish" should be a
role-attribute, and I don't understand why that would be a good idea.
Presumably, we don't want unprivileged users to be able to fire up
logical replication because that involves making connections to other
systems from the PostgreSQL operating system user's account, and that
should be a privileged operation. But that's the subscriber side, not
the publisher side.
I don't otherwise follow Stephen's argument. It seems like he's
complaining that PUBLISH might give more access to the relation than
SELECT, but, uh, that's what granting additional privileges does in
general, by definition. Mostly we consider that a feature, not a bug.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 14/03/17 19:47, Robert Haas wrote:
On Tue, Mar 14, 2017 at 2:41 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:My understanding of what Shephen is proposing is, you have "ownerA" of
tableA and "ownerB" of tableB, then you want role "publishe"r to be able
to publish those, so you simply grant it the "ownerA" and "ownerB"
roles. Obviously that might is many situations mean that the "publisher"
role potentially also gets sweeping privileges to other tables which may
not be desirable.I didn't hear Stephen propose that "publish" should be a
role-attribute, and I don't understand why that would be a good idea.
Presumably, we don't want unprivileged users to be able to fire up
logical replication because that involves making connections to other
systems from the PostgreSQL operating system user's account, and that
should be a privileged operation. But that's the subscriber side, not
the publisher side.I don't otherwise follow Stephen's argument. It seems like he's
complaining that PUBLISH might give more access to the relation than
SELECT, but, uh, that's what granting additional privileges does in
general, by definition. Mostly we consider that a feature, not a bug.
Not what I mean - owner should be able to publish table. If you are
granted role of the owner you can do what owner can no? That's how I
understand Stephen's proposal.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 14/03/17 19:49, Petr Jelinek wrote:
On 14/03/17 19:47, Robert Haas wrote:
On Tue, Mar 14, 2017 at 2:41 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:My understanding of what Shephen is proposing is, you have "ownerA" of
tableA and "ownerB" of tableB, then you want role "publishe"r to be able
to publish those, so you simply grant it the "ownerA" and "ownerB"
roles. Obviously that might is many situations mean that the "publisher"
role potentially also gets sweeping privileges to other tables which may
not be desirable.I didn't hear Stephen propose that "publish" should be a
role-attribute, and I don't understand why that would be a good idea.
Presumably, we don't want unprivileged users to be able to fire up
logical replication because that involves making connections to other
systems from the PostgreSQL operating system user's account, and that
should be a privileged operation. But that's the subscriber side, not
the publisher side.I don't otherwise follow Stephen's argument. It seems like he's
complaining that PUBLISH might give more access to the relation than
SELECT, but, uh, that's what granting additional privileges does in
general, by definition. Mostly we consider that a feature, not a bug.Not what I mean - owner should be able to publish table. If you are
granted role of the owner you can do what owner can no? That's how I
understand Stephen's proposal.
Note that I am not necessarily saying it's better though, just trying to
explain. It definitely has drawbacks, as in order to grant publish on
one table you might be granting lots of privileges on various objects by
granting the role. So for granularity purposes Peter's PUBLISH privilege
for tables sounds better to me.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Greetings,
* Petr Jelinek (petr.jelinek@2ndquadrant.com) wrote:
On 14/03/17 19:47, Robert Haas wrote:
On Tue, Mar 14, 2017 at 2:41 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:My understanding of what Shephen is proposing is, you have "ownerA" of
tableA and "ownerB" of tableB, then you want role "publishe"r to be able
to publish those, so you simply grant it the "ownerA" and "ownerB"
roles. Obviously that might is many situations mean that the "publisher"
role potentially also gets sweeping privileges to other tables which may
not be desirable.I didn't hear Stephen propose that "publish" should be a
role-attribute, and I don't understand why that would be a good idea.
Presumably, we don't want unprivileged users to be able to fire up
logical replication because that involves making connections to other
systems from the PostgreSQL operating system user's account, and that
should be a privileged operation. But that's the subscriber side, not
the publisher side.I don't otherwise follow Stephen's argument. It seems like he's
complaining that PUBLISH might give more access to the relation than
SELECT, but, uh, that's what granting additional privileges does in
general, by definition. Mostly we consider that a feature, not a bug.Not what I mean - owner should be able to publish table. If you are
granted role of the owner you can do what owner can no? That's how I
understand Stephen's proposal.
Exactly correct, yes. I was not suggesting it be a role attribute.
If we move forward with making PUBLISH a GRANT'able option then I do
worry that people will be surprised that PUBLISH gives you more access
to the table than a straight SELECT does. We have a good deal of
granularity in what a user can GRANT'd to see and PUBLISH completely
ignores all of that.
The way I see PUBLISH, it's another command which is able to read from a
table, not unlike the way COPY works, but we don't have an independent
COPY privilege and the COPY command does actually respect the SELECT
privileges on the table.
Another approach to solving my concern would be to only allow the
publishing of tables by non-owner users who have table-level SELECT
rights (meaning they can see all columns of the table) and which don't
have RLS enabled.
Frankly, though, I really don't buy the argument that there's a serious
use-case for non-owners to be GRANT'd the PUBLISH capability, which
means we would end up burning one of the few remaining privilege bits
for something that isn't going to be used and I don't care for that
either.
Thanks!
Stephen
On Tue, Mar 14, 2017 at 2:56 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
Note that I am not necessarily saying it's better though, just trying to
explain. It definitely has drawbacks, as in order to grant publish on
one table you might be granting lots of privileges on various objects by
granting the role. So for granularity purposes Peter's PUBLISH privilege
for tables sounds better to me.
I get that. If, without the patch, letting user X do operation Y will
require either giving user X membership in a role that has many
privileges, and with the patch, will require only granting a specific
privilege on a specific object, then the latter is obviously far
better from a security point of view.
However, what I'm not clear about is whether this is a situation
that's likely to come up much in practice. I would have thought that
publications and subscriptions would typically be configured by roles
with quite high levels of privilege anyway, in which case the separate
PUBLISH privilege would rarely be used in practice, and might
therefore fail to be worth using up a bit. I might be missing a
plausible scenario in which that's not the case, though.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Greetings,
* Robert Haas (robertmhaas@gmail.com) wrote:
However, what I'm not clear about is whether this is a situation
that's likely to come up much in practice. I would have thought that
publications and subscriptions would typically be configured by roles
with quite high levels of privilege anyway, in which case the separate
PUBLISH privilege would rarely be used in practice, and might
therefore fail to be worth using up a bit. I might be missing a
plausible scenario in which that's not the case, though.
Right, this is part of my concern also.
Further, PUBLISH, as I understand it, is something of a one-time or at
least reasonably rarely done operation. This is quite different from a
SELECT privilege which is used on every query against the table and
which may be GRANT'd to user X today and user Y tomorrow and perhaps
REVOKE'd from user X the next day.
What happens when the PUBLISH right is REVOKE'd from the user who did
the PUBLISH in the first place, for example..?
Thanks!
Stephen
On 14/03/17 20:09, Robert Haas wrote:
On Tue, Mar 14, 2017 at 2:56 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:Note that I am not necessarily saying it's better though, just trying to
explain. It definitely has drawbacks, as in order to grant publish on
one table you might be granting lots of privileges on various objects by
granting the role. So for granularity purposes Peter's PUBLISH privilege
for tables sounds better to me.I get that. If, without the patch, letting user X do operation Y will
require either giving user X membership in a role that has many
privileges, and with the patch, will require only granting a specific
privilege on a specific object, then the latter is obviously far
better from a security point of view.However, what I'm not clear about is whether this is a situation
that's likely to come up much in practice. I would have thought that
publications and subscriptions would typically be configured by roles
with quite high levels of privilege anyway, in which case the separate
PUBLISH privilege would rarely be used in practice, and might
therefore fail to be worth using up a bit. I might be missing a
plausible scenario in which that's not the case, though.
Yeah that's rather hard to say in front. Maybe safest action would be to
give the permission to owners in 10 and revisit special privilege in 11
based on feedback?
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Mar 14, 2017 at 3:37 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
On 14/03/17 20:09, Robert Haas wrote:
On Tue, Mar 14, 2017 at 2:56 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:Note that I am not necessarily saying it's better though, just trying to
explain. It definitely has drawbacks, as in order to grant publish on
one table you might be granting lots of privileges on various objects by
granting the role. So for granularity purposes Peter's PUBLISH privilege
for tables sounds better to me.I get that. If, without the patch, letting user X do operation Y will
require either giving user X membership in a role that has many
privileges, and with the patch, will require only granting a specific
privilege on a specific object, then the latter is obviously far
better from a security point of view.However, what I'm not clear about is whether this is a situation
that's likely to come up much in practice. I would have thought that
publications and subscriptions would typically be configured by roles
with quite high levels of privilege anyway, in which case the separate
PUBLISH privilege would rarely be used in practice, and might
therefore fail to be worth using up a bit. I might be missing a
plausible scenario in which that's not the case, though.Yeah that's rather hard to say in front. Maybe safest action would be to
give the permission to owners in 10 and revisit special privilege in 11
based on feedback?
I think that would be entirely sensible.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/14/17 15:05, Stephen Frost wrote:
Another approach to solving my concern would be to only allow the
publishing of tables by non-owner users who have table-level SELECT
rights
An early version of the logical replication patch set did that. But the
problem is that this way someone with SELECT privilege could include a
table without replica identity index into a publication and thereby
prevent updates to the table. There might be ways to tweak things to
make that work better, but right now it works that way.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/14/17 15:37, Petr Jelinek wrote:
Yeah that's rather hard to say in front. Maybe safest action would be to
give the permission to owners in 10 and revisit special privilege in 11
based on feedback?
I'm fine with that.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/14/17 14:49, Petr Jelinek wrote:
Not what I mean - owner should be able to publish table. If you are
granted role of the owner you can do what owner can no?
I didn't actually know that ownership worked that way. You can grant
the role of an owner to someone, and then that someone has ownership
rights. So that should satisfy a pretty good range of use cases for who
can publish what tables.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
New patch set based on the discussions. I have dropped the PUBLICATION
privilege patch. The patches are also reordered a bit in approximate
decreasing priority order.
0001 Refine rules for altering publication owner
kind of a bug fix
0002 Change logical replication pg_hba.conf use
This was touched upon in the discussion at
</messages/by-id/CAB7nPqRf8eOv15SPQJbC1npJoDWTNPMTNp6AvMN-XWwB53h2Cg@mail.gmail.com>
and seems to have been viewed favorably there.
0003 Add USAGE privilege for publications
a way to control who can subscribe to a publication
0004 Add subscription apply worker privilege checks
This is a prerequisite for the next one (or one like it).
0005 Add CREATE SUBSCRIPTION privilege on databases
Need a way to determine which user can create subscriptions. The
presented approach made sense to me, but maybe there are other ideas.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v2-0001-Refine-rules-for-altering-publication-owner.patchapplication/x-patch; name=v2-0001-Refine-rules-for-altering-publication-owner.patchDownload
From 676e6e26f1b0500907c0cd810969bb015ee548d1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 13 Feb 2017 08:57:45 -0500
Subject: [PATCH v2 1/5] 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 | 34 ++++++++++++++++++++++---------
src/test/regress/expected/publication.out | 8 ++++++++
src/test/regress/sql/publication.sql | 4 ++++
4 files changed, 41 insertions(+), 12 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 @@ <title>Description</title>
</para>
<para>
- 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 <literal>CREATE</literal> privilege on
+ the database. Also, the new owner of a <literal>FOR ALL TABLES</literal>
+ publication must be a superuser. However, a superuser can change the
+ ownership of a publication while circumventing these restrictions.
</para>
<para>
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 04f83e0a2e..d69e39dc5b 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -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 7c4834b213..5a7c0edf7d 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -182,6 +182,14 @@ ALTER PUBLICATION testpub_default RENAME TO testpub_foo;
-- rename back to keep the rest simple
ALTER PUBLICATION testpub_foo RENAME TO testpub_default;
+ALTER PUBLICATION testpub_default OWNER TO regress_publication_user2;
+\dRp testpub_default
+ List of publications
+ Name | Owner | Inserts | Updates | Deletes
+-----------------+---------------------------+---------+---------+---------
+ testpub_default | regress_publication_user2 | t | t | t
+(1 row)
+
DROP PUBLICATION testpub_default;
DROP PUBLICATION testpib_ins_trunct;
DROP PUBLICATION testpub_fortbl;
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 46d275acc5..cff9931a77 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -108,6 +108,10 @@ CREATE PUBLICATION testpub2; -- ok
-- rename back to keep the rest simple
ALTER PUBLICATION testpub_foo RENAME TO 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;
--
2.12.0
v2-0002-Change-logical-replication-pg_hba.conf-use.patchapplication/x-patch; name=v2-0002-Change-logical-replication-pg_hba.conf-use.patchDownload
From 82ef38dbdb2118aaef15f9974167b5518707db2c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 13 Feb 2017 16:50:29 -0500
Subject: [PATCH v2 2/5] Change logical replication pg_hba.conf use
Logical replication no longer uses the "replication" keyword. It just
matches database entries in the normal way. The "replication" keyword
now only applies to physical replication.
---
doc/src/sgml/client-auth.sgml | 2 +-
doc/src/sgml/logical-replication.sgml | 8 +++-----
src/backend/libpq/hba.c | 4 ++--
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index bbd52a5418..d6b8c04edc 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -193,7 +193,7 @@ <title>The <filename>pg_hba.conf</filename> File</title>
members of the role, directly or indirectly, and not just by
virtue of being a superuser.
The value <literal>replication</> specifies that the record
- matches if a replication connection is requested (note that
+ matches if a physical replication connection is requested (note that
replication connections do not specify any particular database).
Otherwise, this is the name of
a specific <productname>PostgreSQL</productname> database.
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index a6c04e923d..6da39d25e3 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -295,11 +295,9 @@ <title>Monitoring</title>
<title>Security</title>
<para>
- Logical replication connections occur in the same way as with physical streaming
- replication. It requires access to be explicitly given using
- <filename>pg_hba.conf</filename>. The role used for the replication
- connection must have the <literal>REPLICATION</literal> attribute. This
- gives a role access to both logical and physical replication.
+ The role used for the replication connection must have
+ the <literal>REPLICATION</literal> attribute. Access for the role must be
+ configured in <filename>pg_hba.conf</filename>.
</para>
<para>
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 3817d249c4..7abcae618d 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -612,9 +612,9 @@ check_db(const char *dbname, const char *role, Oid roleid, List *tokens)
foreach(cell, tokens)
{
tok = lfirst(cell);
- if (am_walsender)
+ if (am_walsender && !am_db_walsender)
{
- /* walsender connections can only match replication keyword */
+ /* physical replication walsender connections can only match replication keyword */
if (token_is_keyword(tok, "replication"))
return true;
}
--
2.12.0
v2-0003-Add-USAGE-privilege-for-publications.patchapplication/x-patch; name=v2-0003-Add-USAGE-privilege-for-publications.patchDownload
From dd96a267f96e21ea390c5de46058c3158a5feef8 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 2 Nov 2016 12:00:00 -0400
Subject: [PATCH v2 3/5] Add USAGE privilege for publications
Add pg_publication.pubacl column and associated infrastructure so that
publications can have privileges. USAGE privilege on the publication is
now required for a connecting subscription to be able to use it.
Previously, any connecting user could use any publication, which was not
unreasonable because that user needs to have the REPLICATION attribute,
which is pretty powerful anyway, but we might want to move away from
that, and this gives finer control.
---
doc/src/sgml/catalogs.sgml | 12 ++
doc/src/sgml/func.sgml | 27 ++++
doc/src/sgml/logical-replication.sgml | 5 +
doc/src/sgml/ref/grant.sgml | 8 ++
doc/src/sgml/ref/revoke.sgml | 6 +
src/backend/catalog/aclchk.c | 213 ++++++++++++++++++++++++++++
src/backend/commands/event_trigger.c | 1 +
src/backend/commands/publicationcmds.c | 2 +
src/backend/parser/gram.y | 8 ++
src/backend/replication/pgoutput/pgoutput.c | 8 ++
src/backend/utils/adt/acl.c | 200 ++++++++++++++++++++++++++
src/bin/pg_dump/dumputils.c | 2 +
src/bin/pg_dump/pg_dump.c | 60 ++++++--
src/bin/pg_dump/pg_dump.h | 4 +
src/bin/psql/describe.c | 8 +-
src/bin/psql/tab-complete.c | 1 +
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_proc.h | 13 ++
src/include/catalog/pg_publication.h | 6 +-
src/include/nodes/parsenodes.h | 1 +
src/include/utils/acl.h | 4 +
src/test/regress/expected/publication.out | 33 +++--
src/test/regress/sql/publication.sql | 4 +
src/test/subscription/t/003_privileges.pl | 72 ++++++++++
24 files changed, 675 insertions(+), 25 deletions(-)
create mode 100644 src/test/subscription/t/003_privileges.pl
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 2c2da2ad8a..5f5fdc31c0 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -5378,6 +5378,18 @@ <title><structname>pg_publication</structname> Columns</title>
<entry>If true, <command>DELETE</command> operations are replicated for
tables in the publication.</entry>
</row>
+
+ <row>
+ <entry><structfield>pubacl</structfield></entry>
+ <entry><type>aclitem[]</type></entry>
+ <entry></entry>
+ <entry>
+ Access privileges; see
+ <xref linkend="sql-grant"> and
+ <xref linkend="sql-revoke">
+ for details
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index a521912317..6cb4cff199 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -16173,6 +16173,21 @@ <title>Access Privilege Inquiry Functions</title>
<entry>does current user have privilege for language</entry>
</row>
<row>
+ <entry><literal><function>has_publication_privilege</function>(<parameter>user</parameter>,
+ <parameter>publication</parameter>,
+ <parameter>privilege</parameter>)</literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>does user have privilege for publication</entry>
+ </row>
+ <row>
+ <entry><literal><function>has_publication_privilege</function>(<parameter>publication</parameter>,
+ <parameter>privilege</parameter>)</literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>does current user have privilege for publication</entry>
+ </row>
+ <row>
<entry><literal><function>has_schema_privilege</function>(<parameter>user</parameter>,
<parameter>schema</parameter>,
<parameter>privilege</parameter>)</literal>
@@ -16306,6 +16321,9 @@ <title>Access Privilege Inquiry Functions</title>
<primary>has_language_privilege</primary>
</indexterm>
<indexterm>
+ <primary>has_publication_privilege</primary>
+ </indexterm>
+ <indexterm>
<primary>has_schema_privilege</primary>
</indexterm>
<indexterm>
@@ -16450,6 +16468,15 @@ <title>Access Privilege Inquiry Functions</title>
</para>
<para>
+ <function>has_publication_privilege</function> checks whether a user
+ can access a publication in a particular way.
+ Its argument possibilities
+ are analogous to <function>has_table_privilege</function>.
+ The desired access privilege type must evaluate to
+ <literal>USAGE</literal>.
+ </para>
+
+ <para>
<function>has_schema_privilege</function> checks whether a user
can access a schema in a particular way.
Its argument possibilities
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 6da39d25e3..4c8d454c9e 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -316,6 +316,11 @@ <title>Security</title>
</para>
<para>
+ To use a publication, the remote user of a subscription connection must
+ have the <literal>USAGE</literal> privilege on the publication.
+ </para>
+
+ <para>
The subscription apply process will run in the local database with the
privileges of a superuser.
</para>
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index 9fb4c2fd7e..e1336e6857 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -67,6 +67,10 @@
ON LARGE OBJECT <replaceable class="PARAMETER">loid</replaceable> [, ...]
TO <replaceable class="PARAMETER">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
+GRANT { USAGE | ALL [ PRIVILEGES ] }
+ ON PUBLICATION <replaceable>publication_name</replaceable> [, ...]
+ TO <replaceable class="PARAMETER">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
+
GRANT { { CREATE | USAGE } [, ...] | ALL [ PRIVILEGES ] }
ON SCHEMA <replaceable>schema_name</replaceable> [, ...]
TO <replaceable class="PARAMETER">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
@@ -368,6 +372,10 @@ <title>GRANT on Database Objects</title>
tables using the server, and also to create, alter, or drop their own
user's user mappings associated with that server.
</para>
+ <para>
+ For publications, this privilege allows a subscription to use the
+ publication.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/revoke.sgml b/doc/src/sgml/ref/revoke.sgml
index ce532543f0..ba1a4e73a8 100644
--- a/doc/src/sgml/ref/revoke.sgml
+++ b/doc/src/sgml/ref/revoke.sgml
@@ -88,6 +88,12 @@
[ CASCADE | RESTRICT ]
REVOKE [ GRANT OPTION FOR ]
+ { USAGE | ALL [ PRIVILEGES ] }
+ ON PUBLICATION <replaceable>publication_name</replaceable> [, ...]
+ FROM { [ GROUP ] <replaceable class="PARAMETER">role_name</replaceable> | PUBLIC } [, ...]
+ [ CASCADE | RESTRICT ]
+
+REVOKE [ GRANT OPTION FOR ]
{ { CREATE | USAGE } [, ...] | ALL [ PRIVILEGES ] }
ON SCHEMA <replaceable>schema_name</replaceable> [, ...]
FROM { [ GROUP ] <replaceable class="PARAMETER">role_name</replaceable> | PUBLIC } [, ...]
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index be86d76a59..e029485c8e 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -109,6 +109,7 @@ static void ExecGrant_Function(InternalGrant *grantStmt);
static void ExecGrant_Language(InternalGrant *grantStmt);
static void ExecGrant_Largeobject(InternalGrant *grantStmt);
static void ExecGrant_Namespace(InternalGrant *grantStmt);
+static void ExecGrant_Publication(InternalGrant *grantStmt);
static void ExecGrant_Tablespace(InternalGrant *grantStmt);
static void ExecGrant_Type(InternalGrant *grantStmt);
@@ -283,6 +284,9 @@ restrict_and_check_grant(bool is_grant, AclMode avail_goptions, bool all_privs,
case ACL_KIND_TYPE:
whole_mask = ACL_ALL_RIGHTS_TYPE;
break;
+ case ACL_KIND_PUBLICATION:
+ whole_mask = ACL_ALL_RIGHTS_PUBLICATION;
+ break;
default:
elog(ERROR, "unrecognized object kind: %d", objkind);
/* not reached, but keep compiler quiet */
@@ -497,6 +501,10 @@ ExecuteGrantStmt(GrantStmt *stmt)
all_privileges = ACL_ALL_RIGHTS_FOREIGN_SERVER;
errormsg = gettext_noop("invalid privilege type %s for foreign server");
break;
+ case ACL_OBJECT_PUBLICATION:
+ all_privileges = ACL_ALL_RIGHTS_PUBLICATION;
+ errormsg = gettext_noop("invalid privilege type %s for publication");
+ break;
default:
elog(ERROR, "unrecognized GrantStmt.objtype: %d",
(int) stmt->objtype);
@@ -594,6 +602,9 @@ ExecGrantStmt_oids(InternalGrant *istmt)
case ACL_OBJECT_NAMESPACE:
ExecGrant_Namespace(istmt);
break;
+ case ACL_OBJECT_PUBLICATION:
+ ExecGrant_Publication(istmt);
+ break;
case ACL_OBJECT_TABLESPACE:
ExecGrant_Tablespace(istmt);
break;
@@ -736,6 +747,15 @@ objectNamesToOids(GrantObjectType objtype, List *objnames)
objects = lappend_oid(objects, srvid);
}
break;
+ case ACL_OBJECT_PUBLICATION:
+ foreach(cell, objnames)
+ {
+ char *pubname = strVal(lfirst(cell));
+ Oid pubid = get_publication_oid(pubname, false);
+
+ objects = lappend_oid(objects, pubid);
+ }
+ break;
default:
elog(ERROR, "unrecognized GrantStmt.objtype: %d",
(int) objtype);
@@ -2936,6 +2956,126 @@ ExecGrant_Namespace(InternalGrant *istmt)
}
static void
+ExecGrant_Publication(InternalGrant *istmt)
+{
+ Relation relation;
+ ListCell *cell;
+
+ if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
+ istmt->privileges = ACL_ALL_RIGHTS_PUBLICATION;
+
+ relation = heap_open(PublicationRelationId, RowExclusiveLock);
+
+ foreach(cell, istmt->objects)
+ {
+ Oid pubId = lfirst_oid(cell);
+ Form_pg_publication pg_publication_tuple;
+ Datum aclDatum;
+ bool isNull;
+ AclMode avail_goptions;
+ AclMode this_privileges;
+ Acl *old_acl;
+ Acl *new_acl;
+ Oid grantorId;
+ Oid ownerId;
+ HeapTuple newtuple;
+ Datum values[Natts_pg_publication];
+ bool nulls[Natts_pg_publication];
+ bool replaces[Natts_pg_publication];
+ int noldmembers;
+ int nnewmembers;
+ Oid *oldmembers;
+ Oid *newmembers;
+ HeapTuple tuple;
+
+ /* Search syscache for pg_publication */
+ tuple = SearchSysCache1(PUBLICATIONOID, ObjectIdGetDatum(pubId));
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "cache lookup failed for publication %u", pubId);
+
+ pg_publication_tuple = (Form_pg_publication) GETSTRUCT(tuple);
+
+ /*
+ * Get owner ID and working copy of existing ACL. If there's no ACL,
+ * substitute the proper default.
+ */
+ ownerId = pg_publication_tuple->pubowner;
+ aclDatum = heap_getattr(tuple, Anum_pg_publication_pubacl,
+ RelationGetDescr(relation), &isNull);
+ if (isNull)
+ {
+ old_acl = acldefault(ACL_OBJECT_PUBLICATION, ownerId);
+ /* There are no old member roles according to the catalogs */
+ noldmembers = 0;
+ oldmembers = NULL;
+ }
+ else
+ {
+ old_acl = DatumGetAclPCopy(aclDatum);
+ /* Get the roles mentioned in the existing ACL */
+ noldmembers = aclmembers(old_acl, &oldmembers);
+ }
+
+ /* Determine ID to do the grant as, and available grant options */
+ select_best_grantor(GetUserId(), istmt->privileges,
+ old_acl, ownerId,
+ &grantorId, &avail_goptions);
+
+ /*
+ * Restrict the privileges to what we can actually grant, and emit the
+ * standards-mandated warning and error messages.
+ */
+ this_privileges =
+ restrict_and_check_grant(istmt->is_grant, avail_goptions,
+ istmt->all_privs, istmt->privileges,
+ pubId, grantorId, ACL_KIND_PUBLICATION,
+ NameStr(pg_publication_tuple->pubname),
+ 0, NULL);
+
+ /*
+ * Generate new ACL.
+ */
+ new_acl = merge_acl_with_grant(old_acl, istmt->is_grant,
+ istmt->grant_option, istmt->behavior,
+ istmt->grantees, this_privileges,
+ grantorId, ownerId);
+
+ /*
+ * We need the members of both old and new ACLs so we can correct the
+ * shared dependency information.
+ */
+ nnewmembers = aclmembers(new_acl, &newmembers);
+
+ /* finished building new ACL value, now insert it */
+ MemSet(values, 0, sizeof(values));
+ MemSet(nulls, false, sizeof(nulls));
+ MemSet(replaces, false, sizeof(replaces));
+
+ replaces[Anum_pg_publication_pubacl - 1] = true;
+ values[Anum_pg_publication_pubacl - 1] = PointerGetDatum(new_acl);
+
+ newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
+ nulls, replaces);
+
+ CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
+
+ /* Update the shared dependency ACL info */
+ updateAclDependencies(PublicationRelationId, pubId, 0,
+ ownerId,
+ noldmembers, oldmembers,
+ nnewmembers, newmembers);
+
+ ReleaseSysCache(tuple);
+ pfree(new_acl);
+
+ /* prevent error when processing duplicate objects */
+ CommandCounterIncrement();
+ }
+
+ heap_close(relation, RowExclusiveLock);
+}
+
+static void
ExecGrant_Tablespace(InternalGrant *istmt)
{
Relation relation;
@@ -4192,6 +4332,67 @@ pg_foreign_server_aclmask(Oid srv_oid, Oid roleid,
}
/*
+ * Exported routine for examining a user's privileges for a publication.
+ */
+AclMode
+pg_publication_aclmask(Oid pub_oid, Oid roleid,
+ AclMode mask, AclMaskHow how)
+{
+ AclMode result;
+ HeapTuple tuple;
+ Datum aclDatum;
+ bool isNull;
+ Acl *acl;
+ Oid ownerId;
+
+ Form_pg_publication pubForm;
+
+ /* Bypass permission checks for superusers */
+ if (superuser_arg(roleid))
+ return mask;
+
+ /*
+ * Must get the publication's tuple from pg_publication
+ */
+ tuple = SearchSysCache1(PUBLICATIONOID, ObjectIdGetDatum(pub_oid));
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("publication with OID %u does not exist",
+ pub_oid)));
+ pubForm = (Form_pg_publication) GETSTRUCT(tuple);
+
+ /*
+ * Normal case: get the publication's ACL from pg_publication
+ */
+ ownerId = pubForm->pubowner;
+
+ aclDatum = SysCacheGetAttr(PUBLICATIONOID, tuple,
+ Anum_pg_publication_pubacl, &isNull);
+ if (isNull)
+ {
+ /* No ACL, so build default ACL */
+ acl = acldefault(ACL_OBJECT_PUBLICATION, ownerId);
+ aclDatum = (Datum) 0;
+ }
+ else
+ {
+ /* detoast rel's ACL if necessary */
+ acl = DatumGetAclP(aclDatum);
+ }
+
+ result = aclmask(acl, roleid, ownerId, mask, how);
+
+ /* if we have a detoasted copy, free it */
+ if (acl && (Pointer) acl != DatumGetPointer(aclDatum))
+ pfree(acl);
+
+ ReleaseSysCache(tuple);
+
+ return result;
+}
+
+/*
* Exported routine for examining a user's privileges for a type.
*/
AclMode
@@ -4502,6 +4703,18 @@ pg_foreign_server_aclcheck(Oid srv_oid, Oid roleid, AclMode mode)
}
/*
+ * Exported routine for checking a user's access privileges to a publication
+ */
+AclResult
+pg_publication_aclcheck(Oid pub_oid, Oid roleid, AclMode mode)
+{
+ if (pg_publication_aclmask(pub_oid, roleid, mode, ACLMASK_ANY) != 0)
+ return ACLCHECK_OK;
+ else
+ return ACLCHECK_NO_PRIV;
+}
+
+/*
* Exported routine for checking a user's access privileges to a type
*/
AclResult
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 346b347ae1..7438cfa59a 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -1199,6 +1199,7 @@ EventTriggerSupportsGrantObjectType(GrantObjectType objtype)
case ACL_OBJECT_LANGUAGE:
case ACL_OBJECT_LARGEOBJECT:
case ACL_OBJECT_NAMESPACE:
+ case ACL_OBJECT_PUBLICATION:
case ACL_OBJECT_TYPE:
return true;
default:
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index d69e39dc5b..6d1f392348 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -212,6 +212,8 @@ CreatePublication(CreatePublicationStmt *stmt)
values[Anum_pg_publication_pubdelete - 1] =
BoolGetDatum(publish_delete);
+ nulls[Anum_pg_publication_pubacl - 1] = true;
+
tup = heap_form_tuple(RelationGetDescr(rel), values, nulls);
/* Insert tuple into catalog. */
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 6316688a88..66b957bde4 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -6769,6 +6769,14 @@ privilege_target:
n->objs = $3;
$$ = n;
}
+ | PUBLICATION name_list
+ {
+ PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
+ n->targtype = ACL_TARGET_OBJECT;
+ n->objtype = ACL_OBJECT_PUBLICATION;
+ n->objs = $2;
+ $$ = n;
+ }
| SCHEMA name_list
{
PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 0ceb4be375..54fb8fa185 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -14,6 +14,8 @@
#include "catalog/pg_publication.h"
+#include "miscadmin.h"
+
#include "replication/logical.h"
#include "replication/logicalproto.h"
#include "replication/origin.h"
@@ -398,6 +400,12 @@ LoadPublications(List *pubnames)
{
char *pubname = (char *) lfirst(lc);
Publication *pub = GetPublicationByName(pubname, false);
+ AclResult aclresult;
+
+ aclresult = pg_publication_aclcheck(pub->oid, GetUserId(), ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_PUBLICATION,
+ get_publication_name(pub->oid));
result = lappend(result, pub);
}
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 35bdfc9a46..5298e3dfe3 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -106,6 +106,8 @@ static Oid convert_function_name(text *functionname);
static AclMode convert_function_priv_string(text *priv_type_text);
static Oid convert_language_name(text *languagename);
static AclMode convert_language_priv_string(text *priv_type_text);
+static Oid convert_publication_name(text *publicationname);
+static AclMode convert_publication_priv_string(text *priv_type_text);
static Oid convert_schema_name(text *schemaname);
static AclMode convert_schema_priv_string(text *priv_type_text);
static Oid convert_server_name(text *servername);
@@ -791,6 +793,10 @@ acldefault(GrantObjectType objtype, Oid ownerId)
world_default = ACL_USAGE;
owner_default = ACL_ALL_RIGHTS_TYPE;
break;
+ case ACL_OBJECT_PUBLICATION:
+ world_default = ACL_NO_RIGHTS;
+ owner_default = ACL_ALL_RIGHTS_PUBLICATION;
+ break;
default:
elog(ERROR, "unrecognized objtype: %d", (int) objtype);
world_default = ACL_NO_RIGHTS; /* keep compiler quiet */
@@ -883,6 +889,9 @@ acldefault_sql(PG_FUNCTION_ARGS)
case 'S':
objtype = ACL_OBJECT_FOREIGN_SERVER;
break;
+ case 'p':
+ objtype = ACL_OBJECT_PUBLICATION;
+ break;
case 'T':
objtype = ACL_OBJECT_TYPE;
break;
@@ -3621,6 +3630,197 @@ convert_language_priv_string(text *priv_type_text)
/*
+ * has_publication_privilege variants
+ * These are all named "has_publication_privilege" at the SQL level.
+ * They take various combinations of publication name, publication OID,
+ * user name, user OID, or implicit user = current_user.
+ *
+ * The result is a boolean value: true if user has the indicated
+ * privilege, false if not, or NULL if object doesn't exist.
+ */
+
+/*
+ * has_publication_privilege_name_name
+ * Check user privileges on a publication given
+ * name username, text publicationname, and text priv name.
+ */
+Datum
+has_publication_privilege_name_name(PG_FUNCTION_ARGS)
+{
+ Name username = PG_GETARG_NAME(0);
+ text *publicationname = PG_GETARG_TEXT_P(1);
+ text *priv_type_text = PG_GETARG_TEXT_P(2);
+ Oid roleid;
+ Oid publicationoid;
+ AclMode mode;
+ AclResult aclresult;
+
+ roleid = get_role_oid_or_public(NameStr(*username));
+ publicationoid = convert_publication_name(publicationname);
+ mode = convert_publication_priv_string(priv_type_text);
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_publication_privilege_name
+ * Check user privileges on a publication given
+ * text publicationname and text priv name.
+ * current_user is assumed
+ */
+Datum
+has_publication_privilege_name(PG_FUNCTION_ARGS)
+{
+ text *publicationname = PG_GETARG_TEXT_P(0);
+ text *priv_type_text = PG_GETARG_TEXT_P(1);
+ Oid roleid;
+ Oid publicationoid;
+ AclMode mode;
+ AclResult aclresult;
+
+ roleid = GetUserId();
+ publicationoid = convert_publication_name(publicationname);
+ mode = convert_publication_priv_string(priv_type_text);
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_publication_privilege_name_id
+ * Check user privileges on a publication given
+ * name usename, publication oid, and text priv name.
+ */
+Datum
+has_publication_privilege_name_id(PG_FUNCTION_ARGS)
+{
+ Name username = PG_GETARG_NAME(0);
+ Oid publicationoid = PG_GETARG_OID(1);
+ text *priv_type_text = PG_GETARG_TEXT_P(2);
+ Oid roleid;
+ AclMode mode;
+ AclResult aclresult;
+
+ roleid = get_role_oid_or_public(NameStr(*username));
+ mode = convert_publication_priv_string(priv_type_text);
+
+ if (!SearchSysCacheExists1(PUBLICATIONOID, ObjectIdGetDatum(publicationoid)))
+ PG_RETURN_NULL();
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_publication_privilege_id
+ * Check user privileges on a publication given
+ * publication oid, and text priv name.
+ * current_user is assumed
+ */
+Datum
+has_publication_privilege_id(PG_FUNCTION_ARGS)
+{
+ Oid publicationoid = PG_GETARG_OID(0);
+ text *priv_type_text = PG_GETARG_TEXT_P(1);
+ Oid roleid;
+ AclMode mode;
+ AclResult aclresult;
+
+ roleid = GetUserId();
+ mode = convert_publication_priv_string(priv_type_text);
+
+ if (!SearchSysCacheExists1(PUBLICATIONOID, ObjectIdGetDatum(publicationoid)))
+ PG_RETURN_NULL();
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_publication_privilege_id_name
+ * Check user privileges on a publication given
+ * roleid, text publicationname, and text priv name.
+ */
+Datum
+has_publication_privilege_id_name(PG_FUNCTION_ARGS)
+{
+ Oid roleid = PG_GETARG_OID(0);
+ text *publicationname = PG_GETARG_TEXT_P(1);
+ text *priv_type_text = PG_GETARG_TEXT_P(2);
+ Oid publicationoid;
+ AclMode mode;
+ AclResult aclresult;
+
+ publicationoid = convert_publication_name(publicationname);
+ mode = convert_publication_priv_string(priv_type_text);
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * has_publication_privilege_id_id
+ * Check user privileges on a publication given
+ * roleid, publication oid, and text priv name.
+ */
+Datum
+has_publication_privilege_id_id(PG_FUNCTION_ARGS)
+{
+ Oid roleid = PG_GETARG_OID(0);
+ Oid publicationoid = PG_GETARG_OID(1);
+ text *priv_type_text = PG_GETARG_TEXT_P(2);
+ AclMode mode;
+ AclResult aclresult;
+
+ mode = convert_publication_priv_string(priv_type_text);
+
+ if (!SearchSysCacheExists1(PUBLICATIONOID, ObjectIdGetDatum(publicationoid)))
+ PG_RETURN_NULL();
+
+ aclresult = pg_publication_aclcheck(publicationoid, roleid, mode);
+
+ PG_RETURN_BOOL(aclresult == ACLCHECK_OK);
+}
+
+/*
+ * Support routines for has_publication_privilege family.
+ */
+
+/*
+ * Given a publication name expressed as a string, look it up and return Oid
+ */
+static Oid
+convert_publication_name(text *publicationname)
+{
+ char *pubname = text_to_cstring(publicationname);
+
+ return get_publication_oid(pubname, false);
+}
+
+/*
+ * convert_publication_priv_string
+ * Convert text string to AclMode value.
+ */
+static AclMode
+convert_publication_priv_string(text *priv_type_text)
+{
+ static const priv_map publication_priv_map[] = {
+ {"USAGE", ACL_USAGE},
+ {"USAGE WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_USAGE)},
+ {NULL, 0}
+ };
+
+ return convert_any_priv_string(priv_type_text, publication_priv_map);
+}
+
+
+/*
* has_schema_privilege variants
* These are all named "has_schema_privilege" at the SQL level.
* They take various combinations of schema name, schema OID,
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index b41f2b9125..2e49d5d12a 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -520,6 +520,8 @@ do { \
CONVERT_PRIV('X', "EXECUTE");
else if (strcmp(type, "LANGUAGE") == 0)
CONVERT_PRIV('U', "USAGE");
+ else if (strcmp(type, "PUBLICATION") == 0)
+ CONVERT_PRIV('U', "USAGE");
else if (strcmp(type, "SCHEMA") == 0)
{
CONVERT_PRIV('C', "CREATE");
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e67171dccb..7d31d46cbd 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3349,7 +3349,11 @@ dumpPolicy(Archive *fout, PolicyInfo *polinfo)
void
getPublications(Archive *fout)
{
- PQExpBuffer query;
+ PQExpBuffer acl_subquery = createPQExpBuffer();
+ PQExpBuffer racl_subquery = createPQExpBuffer();
+ PQExpBuffer initacl_subquery = createPQExpBuffer();
+ PQExpBuffer initracl_subquery = createPQExpBuffer();
+ PQExpBuffer query = createPQExpBuffer();
PGresult *res;
PublicationInfo *pubinfo;
int i_tableoid;
@@ -3360,24 +3364,46 @@ getPublications(Archive *fout)
int i_pubinsert;
int i_pubupdate;
int i_pubdelete;
+ int i_pubacl;
+ int i_rpubacl;
+ int i_initpubacl;
+ int i_initrpubacl;
int i,
ntups;
+
if (fout->remoteVersion < 100000)
return;
- query = createPQExpBuffer();
-
- resetPQExpBuffer(query);
+ buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
+ initracl_subquery, "p.pubacl", "p.pubowner", "'p'",
+ fout->dopt->binary_upgrade);
/* Get the publications. */
appendPQExpBuffer(query,
"SELECT p.tableoid, p.oid, p.pubname, "
+ "%s AS pubacl, "
+ "%s AS rpubacl, "
+ "%s AS initpubacl, "
+ "%s AS initrpubacl, "
"(%s p.pubowner) AS rolname, "
"p.puballtables, p.pubinsert, p.pubupdate, p.pubdelete "
- "FROM pg_catalog.pg_publication p",
+ "FROM pg_catalog.pg_publication p "
+ "LEFT JOIN pg_init_privs pip ON "
+ "(p.oid = pip.objoid "
+ "AND pip.classoid = 'pg_publication'::regclass "
+ "AND pip.objsubid = 0) ",
+ acl_subquery->data,
+ racl_subquery->data,
+ initacl_subquery->data,
+ initracl_subquery->data,
username_subquery);
+ destroyPQExpBuffer(acl_subquery);
+ destroyPQExpBuffer(racl_subquery);
+ destroyPQExpBuffer(initacl_subquery);
+ destroyPQExpBuffer(initracl_subquery);
+
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
ntups = PQntuples(res);
@@ -3390,6 +3416,10 @@ getPublications(Archive *fout)
i_pubinsert = PQfnumber(res, "pubinsert");
i_pubupdate = PQfnumber(res, "pubupdate");
i_pubdelete = PQfnumber(res, "pubdelete");
+ i_pubacl = PQfnumber(res, "pubacl");
+ i_rpubacl = PQfnumber(res, "rpubacl");
+ i_initpubacl = PQfnumber(res, "initpubacl");
+ i_initrpubacl = PQfnumber(res, "initrpubacl");
pubinfo = pg_malloc(ntups * sizeof(PublicationInfo));
@@ -3410,6 +3440,10 @@ getPublications(Archive *fout)
(strcmp(PQgetvalue(res, i, i_pubupdate), "t") == 0);
pubinfo[i].pubdelete =
(strcmp(PQgetvalue(res, i, i_pubdelete), "t") == 0);
+ pubinfo[i].pubacl = pg_strdup(PQgetvalue(res, i, i_pubacl));
+ pubinfo[i].rpubacl = pg_strdup(PQgetvalue(res, i, i_rpubacl));
+ pubinfo[i].initpubacl = pg_strdup(PQgetvalue(res, i, i_initpubacl));
+ pubinfo[i].initrpubacl = pg_strdup(PQgetvalue(res, i, i_initrpubacl));
if (strlen(pubinfo[i].rolname) == 0)
write_msg(NULL, "WARNING: owner of publication \"%s\" appears to be invalid\n",
@@ -3430,6 +3464,7 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
DumpOptions *dopt = fout->dopt;
PQExpBuffer delq;
PQExpBuffer query;
+ char *qpubname;
if (dopt->dataOnly)
return;
@@ -3437,11 +3472,11 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
delq = createPQExpBuffer();
query = createPQExpBuffer();
- appendPQExpBuffer(delq, "DROP PUBLICATION %s;\n",
- fmtId(pubinfo->dobj.name));
+ qpubname = pg_strdup(fmtId(pubinfo->dobj.name));
- appendPQExpBuffer(query, "CREATE PUBLICATION %s",
- fmtId(pubinfo->dobj.name));
+ appendPQExpBuffer(delq, "DROP PUBLICATION %s;\n", qpubname);
+
+ appendPQExpBuffer(query, "CREATE PUBLICATION %s", qpubname);
if (pubinfo->puballtables)
appendPQExpBufferStr(query, " FOR ALL TABLES");
@@ -3474,6 +3509,13 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
NULL, 0,
NULL, NULL);
+ if (pubinfo->dobj.dump & DUMP_COMPONENT_ACL)
+ dumpACL(fout, pubinfo->dobj.catId, pubinfo->dobj.dumpId, "PUBLICATION",
+ qpubname, NULL, pubinfo->dobj.name,
+ NULL, pubinfo->rolname, pubinfo->pubacl, pubinfo->rpubacl,
+ pubinfo->initpubacl, pubinfo->initrpubacl);
+
+ free(qpubname);
destroyPQExpBuffer(delq);
destroyPQExpBuffer(query);
}
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index a466527ec6..533ac24ebf 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -583,6 +583,10 @@ typedef struct _PublicationInfo
bool pubinsert;
bool pubupdate;
bool pubdelete;
+ char *pubacl;
+ char *rpubacl;
+ char *initpubacl;
+ char *initrpubacl;
} PublicationInfo;
/*
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 61a3e2a848..6c697ce448 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -5003,7 +5003,9 @@ describePublications(const char *pattern)
printfPQExpBuffer(&buf,
"SELECT oid, pubname, puballtables, pubinsert,\n"
- " pubupdate, pubdelete\n"
+ " pubupdate, pubdelete, ");
+ printACLColumn(&buf, "pubacl");
+ appendPQExpBuffer(&buf,
"FROM pg_catalog.pg_publication\n");
processSQLNamePattern(pset.db, &buf, pattern, false, false,
@@ -5022,7 +5024,7 @@ describePublications(const char *pattern)
for (i = 0; i < PQntuples(res); i++)
{
const char align = 'l';
- int ncols = 3;
+ int ncols = 4;
int nrows = 1;
int tables = 0;
PGresult *tabres;
@@ -5041,10 +5043,12 @@ describePublications(const char *pattern)
printTableAddHeader(&cont, gettext_noop("Inserts"), true, align);
printTableAddHeader(&cont, gettext_noop("Updates"), true, align);
printTableAddHeader(&cont, gettext_noop("Deletes"), true, align);
+ printTableAddHeader(&cont, gettext_noop("Access privileges"), true, align);
printTableAddCell(&cont, PQgetvalue(res, i, 3), false, false);
printTableAddCell(&cont, PQgetvalue(res, i, 4), false, false);
printTableAddCell(&cont, PQgetvalue(res, i, 5), false, false);
+ printTableAddCell(&cont, PQgetvalue(res, i, 6), false, false);
if (puballtables)
printfPQExpBuffer(&buf,
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index e8458e939e..84633559c0 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2800,6 +2800,7 @@ psql_completion(const char *text, int start, int end)
" UNION SELECT 'FUNCTION'"
" UNION SELECT 'LANGUAGE'"
" UNION SELECT 'LARGE OBJECT'"
+ " UNION SELECT 'PUBLICATION'"
" UNION SELECT 'SCHEMA'"
" UNION SELECT 'SEQUENCE'"
" UNION SELECT 'TABLE'"
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index b24e3953a1..eb2cb8d862 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 201703151
+#define CATALOG_VERSION_NO 201703155
#endif
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 3d5d866071..990ce2bf33 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3611,6 +3611,19 @@ DESCR("current user privilege on language by language name");
DATA(insert OID = 2267 ( has_language_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 2 0 16 "26 25" _null_ _null_ _null_ _null_ _null_ has_language_privilege_id _null_ _null_ _null_ ));
DESCR("current user privilege on language by language oid");
+DATA(insert OID = 4001 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "19 25 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_name_name _null_ _null_ _null_ ));
+DESCR("user privilege on publication by username, publication name");
+DATA(insert OID = 4002 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "19 26 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_name_id _null_ _null_ _null_ ));
+DESCR("user privilege on publication by username, publication oid");
+DATA(insert OID = 4003 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "26 25 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_id_name _null_ _null_ _null_ ));
+DESCR("user privilege on publication by user oid, publication name");
+DATA(insert OID = 4004 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "26 26 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_id_id _null_ _null_ _null_ ));
+DESCR("user privilege on publication by user oid, publication oid");
+DATA(insert OID = 4005 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 2 0 16 "25 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_name _null_ _null_ _null_ ));
+DESCR("current user privilege on publication by publication name");
+DATA(insert OID = 4006 ( has_publication_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 2 0 16 "26 25" _null_ _null_ _null_ _null_ _null_ has_publication_privilege_id _null_ _null_ _null_ ));
+DESCR("current user privilege on publication by publication oid");
+
DATA(insert OID = 2268 ( has_schema_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "19 25 25" _null_ _null_ _null_ _null_ _null_ has_schema_privilege_name_name _null_ _null_ _null_ ));
DESCR("user privilege on schema by username, schema name");
DATA(insert OID = 2269 ( has_schema_privilege PGNSP PGUID 12 1 0 0 0 f f f f t f s s 3 0 16 "19 26 25" _null_ _null_ _null_ _null_ _null_ has_schema_privilege_name_id _null_ _null_ _null_ ));
diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h
index f3c4f3932b..bce81c373f 100644
--- a/src/include/catalog/pg_publication.h
+++ b/src/include/catalog/pg_publication.h
@@ -49,6 +49,9 @@ CATALOG(pg_publication,6104)
/* true if deletes are published */
bool pubdelete;
+#ifdef CATALOG_VARLEN /* variable-length fields start here */
+ aclitem pubacl[1]; /* access permissions */
+#endif
} FormData_pg_publication;
/* ----------------
@@ -63,13 +66,14 @@ typedef FormData_pg_publication *Form_pg_publication;
* ----------------
*/
-#define Natts_pg_publication 6
+#define Natts_pg_publication 7
#define Anum_pg_publication_pubname 1
#define Anum_pg_publication_pubowner 2
#define Anum_pg_publication_puballtables 3
#define Anum_pg_publication_pubinsert 4
#define Anum_pg_publication_pubupdate 5
#define Anum_pg_publication_pubdelete 6
+#define Anum_pg_publication_pubacl 7
typedef struct PublicationActions
{
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index d576523f6a..276489e556 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1782,6 +1782,7 @@ typedef enum GrantObjectType
ACL_OBJECT_LANGUAGE, /* procedural language */
ACL_OBJECT_LARGEOBJECT, /* largeobject */
ACL_OBJECT_NAMESPACE, /* namespace */
+ ACL_OBJECT_PUBLICATION, /* publication */
ACL_OBJECT_TABLESPACE, /* tablespace */
ACL_OBJECT_TYPE /* type */
} GrantObjectType;
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 0d118525c9..5a5836fd4a 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -158,6 +158,7 @@ typedef ArrayType Acl;
#define ACL_ALL_RIGHTS_NAMESPACE (ACL_USAGE|ACL_CREATE)
#define ACL_ALL_RIGHTS_TABLESPACE (ACL_CREATE)
#define ACL_ALL_RIGHTS_TYPE (ACL_USAGE)
+#define ACL_ALL_RIGHTS_PUBLICATION (ACL_USAGE)
/* operation codes for pg_*_aclmask */
typedef enum
@@ -273,6 +274,8 @@ extern AclMode pg_foreign_data_wrapper_aclmask(Oid fdw_oid, Oid roleid,
AclMode mask, AclMaskHow how);
extern AclMode pg_foreign_server_aclmask(Oid srv_oid, Oid roleid,
AclMode mask, AclMaskHow how);
+extern AclMode pg_publication_aclmask(Oid pub_oid, Oid roleid,
+ AclMode mask, AclMaskHow how);
extern AclMode pg_type_aclmask(Oid type_oid, Oid roleid,
AclMode mask, AclMaskHow how);
@@ -290,6 +293,7 @@ extern AclResult pg_namespace_aclcheck(Oid nsp_oid, Oid roleid, AclMode mode);
extern AclResult pg_tablespace_aclcheck(Oid spc_oid, Oid roleid, AclMode mode);
extern AclResult pg_foreign_data_wrapper_aclcheck(Oid fdw_oid, Oid roleid, AclMode mode);
extern AclResult pg_foreign_server_aclcheck(Oid srv_oid, Oid roleid, AclMode mode);
+extern AclResult pg_publication_aclcheck(Oid pub_oid, Oid roleid, AclMode mode);
extern AclResult pg_type_aclcheck(Oid type_oid, Oid roleid, AclMode mode);
extern void aclcheck_error(AclResult aclerr, AclObjectKind objectkind,
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 5a7c0edf7d..f72bb1b2d3 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -76,10 +76,10 @@ ERROR: relation "testpub_tbl1" is already member of publication "testpub_fortbl
CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1;
ERROR: publication "testpub_fortbl" already exists
\dRp+ testpub_fortbl
- Publication testpub_fortbl
- Inserts | Updates | Deletes
----------+---------+---------
- t | t | t
+ Publication testpub_fortbl
+ Inserts | Updates | Deletes | Access privileges
+---------+---------+---------+-------------------
+ t | t | t |
Tables:
"pub_test.testpub_nopk"
"public.testpub_tbl1"
@@ -117,10 +117,10 @@ Publications:
"testpub_fortbl"
\dRp+ testpub_default
- Publication testpub_default
- Inserts | Updates | Deletes
----------+---------+---------
- t | t | t
+ Publication testpub_default
+ Inserts | Updates | Deletes | Access privileges
+---------+---------+---------+-------------------
+ t | t | t |
Tables:
"pub_test.testpub_nopk"
"public.testpub_tbl1"
@@ -161,10 +161,10 @@ REVOKE CREATE ON DATABASE regression FROM regress_publication_user2;
DROP VIEW testpub_view;
DROP TABLE testpub_tbl1;
\dRp+ testpub_default
- Publication testpub_default
- Inserts | Updates | Deletes
----------+---------+---------
- t | t | t
+ Publication testpub_default
+ Inserts | Updates | Deletes | Access privileges
+---------+---------+---------+-------------------
+ t | t | t |
(1 row)
-- fail - must be owner of publication
@@ -190,6 +190,15 @@ ALTER PUBLICATION testpub_default OWNER TO regress_publication_user2;
testpub_default | regress_publication_user2 | t | t | t
(1 row)
+GRANT USAGE ON PUBLICATION testpub_default TO regress_publication_user;
+\dRp+ testpub_default
+ Publication testpub_default
+ Inserts | Updates | Deletes | Access privileges
+---------+---------+---------+-------------------------------------------------------
+ t | t | t | regress_publication_user2=U/regress_publication_user2+
+ | | | regress_publication_user=U/regress_publication_user2
+(1 row)
+
DROP PUBLICATION testpub_default;
DROP PUBLICATION testpib_ins_trunct;
DROP PUBLICATION testpub_fortbl;
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index cff9931a77..f237c3faa9 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -112,6 +112,10 @@ CREATE PUBLICATION testpub2; -- ok
\dRp testpub_default
+GRANT USAGE ON PUBLICATION testpub_default TO regress_publication_user;
+
+\dRp+ testpub_default
+
DROP PUBLICATION testpub_default;
DROP PUBLICATION testpib_ins_trunct;
DROP PUBLICATION testpub_fortbl;
diff --git a/src/test/subscription/t/003_privileges.pl b/src/test/subscription/t/003_privileges.pl
new file mode 100644
index 0000000000..86fd866e28
--- /dev/null
+++ b/src/test/subscription/t/003_privileges.pl
@@ -0,0 +1,72 @@
+# Tests of privileges for logical replication
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More tests => 2;
+
+my $node_publisher = get_new_node('publisher');
+$node_publisher->init(allows_streaming => 'logical');
+$node_publisher->start;
+
+my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+
+my $node_subscriber = get_new_node('subscriber');
+$node_subscriber->init(allows_streaming => 'logical');
+$node_subscriber->start;
+
+$node_publisher->safe_psql('postgres', qq(
+CREATE USER test_user1;
+CREATE USER test_user2 REPLICATION;
+GRANT CREATE ON DATABASE postgres TO test_user1;
+
+SET ROLE test_user1;
+CREATE TABLE test1 (a int PRIMARY KEY, b text);
+CREATE PUBLICATION mypub1 FOR TABLE test1;
+));
+
+my $appname = 'tap_sub';
+
+$node_subscriber->safe_psql('postgres', qq(
+CREATE TABLE test1 (a int PRIMARY KEY, b text);
+CREATE SUBSCRIPTION mysub1 CONNECTION '$publisher_connstr user=test_user2 application_name=$appname' PUBLICATION mypub1;
+));
+
+$node_publisher->safe_psql('postgres', qq(
+SET ROLE test_user1;
+INSERT INTO test1 VALUES (1, 'one');
+));
+
+my $log = TestLib::slurp_file($node_publisher->logfile);
+like($log, qr/permission denied for publication mypub1/, "permission denied on publication");
+
+$node_publisher->safe_psql('postgres', qq(
+SET ROLE test_user1;
+GRANT USAGE ON PUBLICATION mypub1 TO test_user2;
+));
+
+# drop and recreate subscription so it sees the newly granted
+# privileges
+$node_subscriber->safe_psql('postgres', qq(
+DROP SUBSCRIPTION mysub1;
+CREATE SUBSCRIPTION mysub1 CONNECTION '$publisher_connstr user=test_user2 application_name=$appname' PUBLICATION mypub1;
+));
+
+$node_publisher->safe_psql('postgres', qq(
+SET ROLE test_user1;
+INSERT INTO test1 VALUES (2, 'two');
+));
+
+my $caughtup_query =
+ "SELECT pg_current_wal_location() <= replay_location FROM pg_stat_replication WHERE application_name = '$appname';";
+$node_publisher->poll_query_until('postgres', $caughtup_query)
+ or die "Timed out while waiting for subscriber to catch up";
+
+my $result = $node_subscriber->safe_psql('postgres', qq(
+SELECT a, b FROM test1;
+));
+
+is($result, '2|two', 'replication catches up after privileges granted');
+
+$node_subscriber->stop;
+$node_publisher->stop;
--
2.12.0
v2-0004-Add-subscription-apply-worker-privilege-checks.patchapplication/x-patch; name=v2-0004-Add-subscription-apply-worker-privilege-checks.patchDownload
From 44e7b9e5ba4449e10c0cd788534ac38f222fc68f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 13 Feb 2017 16:28:36 -0500
Subject: [PATCH v2 4/5] Add subscription apply worker privilege checks
The subscription apply worker now checks INSERT/UPDATE/DELETE privileges
on a table before writing to it. This was previously not checked, but
was not necessary since a subscription owner had to be a superuser. But
we want to allow other users to own subscriptions.
---
doc/src/sgml/logical-replication.sgml | 4 +++-
src/backend/replication/logical/relation.c | 24 ++++++++++++++++++++++++
src/backend/replication/logical/worker.c | 3 +++
src/include/replication/logicalrelation.h | 5 +++++
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 4c8d454c9e..89b4335d00 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -322,7 +322,9 @@ <title>Security</title>
<para>
The subscription apply process will run in the local database with the
- privileges of a superuser.
+ privileges of the subscription owner. The subscription owner, if it is not
+ a superuser, therefore needs to have appropriate privileges on the tables
+ the subscription is operating on.
</para>
<para>
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index d8dc0c7194..4faee8323c 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -19,6 +19,7 @@
#include "access/heapam.h"
#include "access/sysattr.h"
#include "catalog/namespace.h"
+#include "miscadmin.h"
#include "nodes/makefuncs.h"
#include "replication/logicalrelation.h"
#include "replication/worker_internal.h"
@@ -78,6 +79,18 @@ logicalrep_relmap_invalidate_cb(Datum arg, Oid reloid)
}
/*
+ * Syscache invalidation callback for our relation map cache.
+ *
+ * Same as the relcache callback, except that we just invalidate all cache
+ * entries all the time.
+ */
+static void
+logicalrep_relmap_syscache_invalidate_cb(Datum arg, int cacheid, uint32 hashvalue)
+{
+ logicalrep_relmap_invalidate_cb(arg, InvalidOid);
+}
+
+/*
* Initialize the relation map cache.
*/
static void
@@ -113,6 +126,8 @@ logicalrep_relmap_init()
/* Watch for invalidation events. */
CacheRegisterRelcacheCallback(logicalrep_relmap_invalidate_cb,
(Datum) 0);
+ CacheRegisterSyscacheCallback(AUTHOID, logicalrep_relmap_syscache_invalidate_cb,
+ (Datum) 0);
CacheRegisterSyscacheCallback(TYPEOID, logicalrep_typmap_invalidate_cb,
(Datum) 0);
}
@@ -277,6 +292,15 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
remoterel->nspname, remoterel->relname)));
/*
+ * Cache ACL results. If they are changed while the worker is active,
+ * the relcache and syscache invalidation will ensure that we get here
+ * again to recompute this.
+ */
+ entry->insert_aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_INSERT);
+ entry->update_aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_UPDATE);
+ entry->delete_aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_DELETE);
+
+ /*
* Build the mapping of local attribute numbers to remote attribute
* numbers and validate that we don't miss any replicated columns
* as that would result in potentially unwanted data loss.
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index c3e54af259..7534702df8 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -515,6 +515,7 @@ apply_handle_insert(StringInfo s)
relid = logicalrep_read_insert(s, &newtup);
rel = logicalrep_rel_open(relid, RowExclusiveLock);
+ aclcheck_error(rel->insert_aclresult, ACL_KIND_CLASS, get_rel_name(rel->localreloid));
/* Initialize the executor state. */
estate = create_estate_for_relation(rel);
@@ -607,6 +608,7 @@ apply_handle_update(StringInfo s)
relid = logicalrep_read_update(s, &has_oldtup, &oldtup,
&newtup);
rel = logicalrep_rel_open(relid, RowExclusiveLock);
+ aclcheck_error(rel->update_aclresult, ACL_KIND_CLASS, get_rel_name(rel->localreloid));
/* Check if we can do the update. */
check_relation_updatable(rel);
@@ -716,6 +718,7 @@ apply_handle_delete(StringInfo s)
relid = logicalrep_read_delete(s, &oldtup);
rel = logicalrep_rel_open(relid, RowExclusiveLock);
+ aclcheck_error(rel->delete_aclresult, ACL_KIND_CLASS, get_rel_name(rel->localreloid));
/* Check if we can do the delete. */
check_relation_updatable(rel);
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index 7fb7fbfb4d..f9d5144f61 100644
--- a/src/include/replication/logicalrelation.h
+++ b/src/include/replication/logicalrelation.h
@@ -25,6 +25,11 @@ typedef struct LogicalRepRelMapEntry
* remote ones */
bool updatable; /* Can apply updates/deletes? */
+ /* Cache of ACL results */
+ AclResult insert_aclresult;
+ AclResult update_aclresult;
+ AclResult delete_aclresult;
+
/* Sync state. */
char state;
XLogRecPtr statelsn;
--
2.12.0
v2-0005-Add-CREATE-SUBSCRIPTION-privilege-on-databases.patchapplication/x-patch; name=v2-0005-Add-CREATE-SUBSCRIPTION-privilege-on-databases.patchDownload
From 3b3a5e1d6d3d24b063151975eb495cfdbcfc71a2 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 13 Feb 2017 15:42:29 -0500
Subject: [PATCH v2 5/5] Add CREATE SUBSCRIPTION privilege on databases
This new privilege allows the creation of subscriptions. This was
previously only allowed for superusers.
---
doc/src/sgml/logical-replication.sgml | 3 ++-
doc/src/sgml/ref/create_subscription.sgml | 6 +++++
doc/src/sgml/ref/grant.sgml | 12 +++++++++-
src/backend/catalog/aclchk.c | 4 ++++
src/backend/commands/subscriptioncmds.c | 38 +++++++++++++++++++-----------
src/backend/parser/gram.y | 6 +++++
src/backend/utils/adt/acl.c | 9 +++++++
src/bin/pg_dump/dumputils.c | 2 ++
src/include/nodes/parsenodes.h | 3 ++-
src/include/utils/acl.h | 5 ++--
src/test/regress/expected/subscription.out | 14 ++++++++++-
src/test/regress/sql/subscription.sql | 14 ++++++++++-
12 files changed, 95 insertions(+), 21 deletions(-)
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 89b4335d00..e49f0ef05b 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -312,7 +312,8 @@ <title>Security</title>
</para>
<para>
- To create a subscription, the user must be a superuser.
+ To create a subscription, the user must have the <literal>CREATE
+ SUBSCRIPTION</literal> privilege in the database.
</para>
<para>
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 9bed26219c..94dec39228 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -136,6 +136,12 @@ <title>Parameters</title>
<title>Notes</title>
<para>
+ To create a subscription, the invoking user must have the
+ <literal>CREATE SUBSCRIPTION</> privilege for the current database. (Of
+ course, superusers bypass this check.)
+ </para>
+
+ <para>
See <xref linkend="streaming-replication-authentication"> for details on
how to configure access control between the subscription and the
publication instance.
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index e1336e6857..9ca0eadb5c 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -38,7 +38,7 @@
| ALL SEQUENCES IN SCHEMA <replaceable class="PARAMETER">schema_name</replaceable> [, ...] }
TO <replaceable class="PARAMETER">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
-GRANT { { CREATE | CONNECT | TEMPORARY | TEMP } [, ...] | ALL [ PRIVILEGES ] }
+GRANT { { CREATE | CONNECT | TEMPORARY | TEMP | CREATE SUBSCRIPTION } [, ...] | ALL [ PRIVILEGES ] }
ON DATABASE <replaceable>database_name</replaceable> [, ...]
TO <replaceable class="PARAMETER">role_specification</replaceable> [, ...] [ WITH GRANT OPTION ]
@@ -300,6 +300,15 @@ <title>GRANT on Database Objects</title>
</varlistentry>
<varlistentry>
+ <term>CREATE SUBSCRIPTION</term>
+ <listitem>
+ <para>
+ For databases, allows new subscriptions to be created within the database.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term>CONNECT</term>
<listitem>
<para>
@@ -542,6 +551,7 @@ <title>Notes</title>
X -- EXECUTE
U -- USAGE
C -- CREATE
+ S -- CREATE SUBSCRIPTION
c -- CONNECT
T -- TEMPORARY
arwdDxt -- ALL PRIVILEGES (for tables, varies for other objects)
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index e029485c8e..4ab6e0b784 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -3362,6 +3362,8 @@ string_to_privilege(const char *privname)
return ACL_CREATE_TEMP;
if (strcmp(privname, "connect") == 0)
return ACL_CONNECT;
+ if (strcmp(privname, "create subscription") == 0)
+ return ACL_CREATE_SUBSCRIPTION;
if (strcmp(privname, "rule") == 0)
return 0; /* ignore old RULE privileges */
ereport(ERROR,
@@ -3399,6 +3401,8 @@ privilege_to_string(AclMode privilege)
return "TEMP";
case ACL_CONNECT:
return "CONNECT";
+ case ACL_CREATE_SUBSCRIPTION:
+ return "CREATE SUBSCRIPTION";
default:
elog(ERROR, "unrecognized privilege: %d", (int) privilege);
}
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 0198e6d75b..c20de68c75 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -26,6 +26,7 @@
#include "catalog/pg_type.h"
#include "catalog/pg_subscription.h"
+#include "commands/dbcommands.h"
#include "commands/defrem.h"
#include "commands/event_trigger.h"
#include "commands/subscriptioncmds.h"
@@ -221,6 +222,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
char originname[NAMEDATALEN];
bool create_slot;
List *publications;
+ AclResult aclresult;
/*
* Parse and check options.
@@ -239,10 +241,11 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
if (create_slot)
PreventTransactionChain(isTopLevel, "CREATE SUBSCRIPTION ... CREATE SLOT");
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to create subscriptions"))));
+ /* must have CREATE SUBSCRIPTION privilege on database */
+ aclresult = pg_database_aclcheck(MyDatabaseId, GetUserId(), ACL_CREATE_SUBSCRIPTION);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_DATABASE,
+ get_database_name(MyDatabaseId));
rel = heap_open(SubscriptionRelationId, RowExclusiveLock);
@@ -609,17 +612,24 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
if (form->subowner == newOwnerId)
return;
- if (!pg_subscription_ownercheck(HeapTupleGetOid(tup), GetUserId()))
- aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_SUBSCRIPTION,
- NameStr(form->subname));
+ 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 subscription \"%s\"",
- NameStr(form->subname)),
- errhint("The owner of an subscription must be a superuser.")));
+ /* Must be owner */
+ if (!pg_subscription_ownercheck(HeapTupleGetOid(tup), GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_SUBSCRIPTION,
+ NameStr(form->subname));
+
+ /* Must be able to become new owner */
+ check_is_member_of_role(GetUserId(), newOwnerId);
+
+ /* New owner must have CREATE SUBSCRIPTION privilege on database */
+ aclresult = pg_database_aclcheck(MyDatabaseId, newOwnerId, ACL_CREATE_SUBSCRIPTION);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_DATABASE,
+ get_database_name(MyDatabaseId));
+ }
form->subowner = newOwnerId;
CatalogTupleUpdate(rel, &tup->t_self, tup);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 66b957bde4..26f8d9bb0c 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -6682,6 +6682,12 @@ privilege: SELECT opt_column_list
n->cols = $2;
$$ = n;
}
+ | CREATE ColId
+ {
+ AccessPriv *n = makeNode(AccessPriv);
+ n->priv_name = psprintf("create %s", $2);
+ $$ = n;
+ }
;
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 5298e3dfe3..e0a3f6e6dd 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -316,6 +316,9 @@ aclparse(const char *s, AclItem *aip)
case ACL_CONNECT_CHR:
read = ACL_CONNECT;
break;
+ case ACL_CREATE_SUBSCRIPTION_CHR:
+ read = ACL_CREATE_SUBSCRIPTION;
+ break;
case 'R': /* ignore old RULE privileges */
read = 0;
break;
@@ -1617,6 +1620,8 @@ convert_priv_string(text *priv_type_text)
return ACL_CREATE_TEMP;
if (pg_strcasecmp(priv_type, "CONNECT") == 0)
return ACL_CONNECT;
+ if (pg_strcasecmp(priv_type, "CREATE SUBSCRIPTION") == 0)
+ return ACL_CREATE_SUBSCRIPTION;
if (pg_strcasecmp(priv_type, "RULE") == 0)
return 0; /* ignore old RULE privileges */
@@ -1713,6 +1718,8 @@ convert_aclright_to_string(int aclright)
return "TEMPORARY";
case ACL_CONNECT:
return "CONNECT";
+ case ACL_CREATE_SUBSCRIPTION:
+ return "CREATE SUBSCRIPTION";
default:
elog(ERROR, "unrecognized aclright: %d", aclright);
return NULL;
@@ -3048,6 +3055,8 @@ convert_database_priv_string(text *priv_type_text)
{"TEMP WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_CREATE_TEMP)},
{"CONNECT", ACL_CONNECT},
{"CONNECT WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_CONNECT)},
+ {"CREATE SUBSCRIPTION", ACL_CREATE_SUBSCRIPTION},
+ {"CREATE SUBSCRIPTION WITH GRANT OPTION", ACL_GRANT_OPTION_FOR(ACL_CREATE_SUBSCRIPTION)},
{NULL, 0}
};
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 2e49d5d12a..c7f10b7bd1 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -532,6 +532,8 @@ do { \
CONVERT_PRIV('C', "CREATE");
CONVERT_PRIV('c', "CONNECT");
CONVERT_PRIV('T', "TEMPORARY");
+ if (remoteVersion >= 100000)
+ CONVERT_PRIV('S', "CREATE SUBSCRIPTION");
}
else if (strcmp(type, "TABLESPACE") == 0)
CONVERT_PRIV('C', "CREATE");
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 276489e556..4ec77e53c2 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -75,7 +75,8 @@ typedef uint32 AclMode; /* a bitmask of privilege bits */
#define ACL_CREATE (1<<9) /* for namespaces and databases */
#define ACL_CREATE_TEMP (1<<10) /* for databases */
#define ACL_CONNECT (1<<11) /* for databases */
-#define N_ACL_RIGHTS 12 /* 1 plus the last 1<<x */
+#define ACL_CREATE_SUBSCRIPTION (1<<12)
+#define N_ACL_RIGHTS 13 /* 1 plus the last 1<<x */
#define ACL_NO_RIGHTS 0
/* Currently, SELECT ... FOR [KEY] UPDATE/SHARE requires UPDATE privileges */
#define ACL_SELECT_FOR_UPDATE ACL_UPDATE
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 5a5836fd4a..a9fdb3250c 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -139,9 +139,10 @@ typedef ArrayType Acl;
#define ACL_CREATE_CHR 'C'
#define ACL_CREATE_TEMP_CHR 'T'
#define ACL_CONNECT_CHR 'c'
+#define ACL_CREATE_SUBSCRIPTION_CHR 'S'
/* string holding all privilege code chars, in order by bitmask position */
-#define ACL_ALL_RIGHTS_STR "arwdDxtXUCTc"
+#define ACL_ALL_RIGHTS_STR "arwdDxtXUCTcS"
/*
* Bitmasks defining "all rights" for each supported object type
@@ -149,7 +150,7 @@ typedef ArrayType Acl;
#define ACL_ALL_RIGHTS_COLUMN (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_REFERENCES)
#define ACL_ALL_RIGHTS_RELATION (ACL_INSERT|ACL_SELECT|ACL_UPDATE|ACL_DELETE|ACL_TRUNCATE|ACL_REFERENCES|ACL_TRIGGER)
#define ACL_ALL_RIGHTS_SEQUENCE (ACL_USAGE|ACL_SELECT|ACL_UPDATE)
-#define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP|ACL_CONNECT)
+#define ACL_ALL_RIGHTS_DATABASE (ACL_CREATE|ACL_CREATE_TEMP|ACL_CONNECT|ACL_CREATE_SUBSCRIPTION)
#define ACL_ALL_RIGHTS_FDW (ACL_USAGE)
#define ACL_ALL_RIGHTS_FOREIGN_SERVER (ACL_USAGE)
#define ACL_ALL_RIGHTS_FUNCTION (ACL_EXECUTE)
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 3471d88ca7..b5aa15336b 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -2,8 +2,9 @@
-- SUBSCRIPTION
--
CREATE ROLE regress_subscription_user LOGIN SUPERUSER;
+CREATE ROLE regress_subscription_user2 LOGIN;
CREATE ROLE regress_subscription_user_dummy LOGIN NOSUPERUSER;
-SET SESSION AUTHORIZATION 'regress_subscription_user';
+SET SESSION AUTHORIZATION regress_subscription_user;
-- fail - no publications
CREATE SUBSCRIPTION testsub CONNECTION 'foo';
ERROR: syntax error at or near ";"
@@ -90,6 +91,17 @@ COMMIT;
BEGIN;
DROP SUBSCRIPTION testsub NODROP SLOT;
COMMIT;
+-- permissions
+set client_min_messages to error;
+SET SESSION AUTHORIZATION regress_subscription_user2;
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (DISABLED, NOCREATE SLOT); -- fail
+ERROR: permission denied for database regression
+SET SESSION AUTHORIZATION regress_subscription_user;
+GRANT CREATE SUBSCRIPTION ON DATABASE regression TO regress_subscription_user2;
+SET SESSION AUTHORIZATION regress_subscription_user2;
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (DISABLED, NOCREATE SLOT); -- ok
+reset client_min_messages;
+DROP SUBSCRIPTION testsub2 NODROP SLOT;
RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
DROP ROLE regress_subscription_user_dummy;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 5c05b14f9e..4b19110abc 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -3,8 +3,9 @@
--
CREATE ROLE regress_subscription_user LOGIN SUPERUSER;
+CREATE ROLE regress_subscription_user2 LOGIN;
CREATE ROLE regress_subscription_user_dummy LOGIN NOSUPERUSER;
-SET SESSION AUTHORIZATION 'regress_subscription_user';
+SET SESSION AUTHORIZATION regress_subscription_user;
-- fail - no publications
CREATE SUBSCRIPTION testsub CONNECTION 'foo';
@@ -65,6 +66,17 @@ CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub
DROP SUBSCRIPTION testsub NODROP SLOT;
COMMIT;
+-- permissions
+set client_min_messages to error;
+SET SESSION AUTHORIZATION regress_subscription_user2;
+CREATE SUBSCRIPTION testsub CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (DISABLED, NOCREATE SLOT); -- fail
+SET SESSION AUTHORIZATION regress_subscription_user;
+GRANT CREATE SUBSCRIPTION ON DATABASE regression TO regress_subscription_user2;
+SET SESSION AUTHORIZATION regress_subscription_user2;
+CREATE SUBSCRIPTION testsub2 CONNECTION 'dbname=doesnotexist' PUBLICATION testpub WITH (DISABLED, NOCREATE SLOT); -- ok
+reset client_min_messages;
+DROP SUBSCRIPTION testsub2 NODROP SLOT;
+
RESET SESSION AUTHORIZATION;
DROP ROLE regress_subscription_user;
DROP ROLE regress_subscription_user_dummy;
--
2.12.0
Hi,
I went over this patch set, don't really have all that much to say
except it looks good for the most part (details inline).
On 16/03/17 02:54, Peter Eisentraut wrote:
New patch set based on the discussions. I have dropped the PUBLICATION
privilege patch. The patches are also reordered a bit in approximate
decreasing priority order.0001 Refine rules for altering publication owner
kind of a bug fix
Agreed, this can be committed as is.
0002 Change logical replication pg_hba.conf use
This was touched upon in the discussion at
</messages/by-id/CAB7nPqRf8eOv15SPQJbC1npJoDWTNPMTNp6AvMN-XWwB53h2Cg@mail.gmail.com>
and seems to have been viewed favorably there.
Seems like a good idea and I think can be committed as well.
0003 Add USAGE privilege for publications
a way to control who can subscribe to a publication
Hmm IIUC this removes ability of REPLICATION role to subscribe to
publications. I am not quite sure I like that.
0004 Add subscription apply worker privilege checks
This is a prerequisite for the next one (or one like it).
0005 Add CREATE SUBSCRIPTION privilege on databases
Need a way to determine which user can create subscriptions. The
presented approach made sense to me, but maybe there are other ideas.
The CREATE SUBSCRIPTION as name of privilege is bit weird but something
like SUBSCRIBE would be more fitting for publish side (to which you
subscriber) so don't really have a better name. I like that the patches
cache the acl result so performance impact should be negligible.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/18/17 09:31, Petr Jelinek wrote:
0003 Add USAGE privilege for publications
a way to control who can subscribe to a publication
Hmm IIUC this removes ability of REPLICATION role to subscribe to
publications. I am not quite sure I like that.
Well, this is kind of the way with all privileges. They take away
abilities by default so you can assign them in a more fine-grained manner.
You can still connect as superuser and do anything you want, if you want
a "quick start" setup.
Right now, any replication user connecting can use any publication.
There is no way to distinguish different table groupings or different
use cases, such as partial replication of some tables that should go
over here, or archiving of some other tables that should go over there.
That's not optimal.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 20/03/17 13:32, Peter Eisentraut wrote:
On 3/18/17 09:31, Petr Jelinek wrote:
0003 Add USAGE privilege for publications
a way to control who can subscribe to a publication
Hmm IIUC this removes ability of REPLICATION role to subscribe to
publications. I am not quite sure I like that.Well, this is kind of the way with all privileges. They take away
abilities by default so you can assign them in a more fine-grained manner.You can still connect as superuser and do anything you want, if you want
a "quick start" setup.Right now, any replication user connecting can use any publication.
There is no way to distinguish different table groupings or different
use cases, such as partial replication of some tables that should go
over here, or archiving of some other tables that should go over there.
That's not optimal.
Hmm but REPLICATION role can do basebackup/consume wal, so how does
giving it limited publication access help? Wouldn't we need some
SUBSCRIPTION role/grant used instead for logical replication connections
instead of REPLICATION for this to make sense?
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/20/17 15:10, Petr Jelinek wrote:
Hmm but REPLICATION role can do basebackup/consume wal, so how does
giving it limited publication access help? Wouldn't we need some
SUBSCRIPTION role/grant used instead for logical replication connections
instead of REPLICATION for this to make sense?
Since we're splitting up the pg_hba.conf setup for logical and physical
connections, it would probably not matter.
But just to think it through, how could we split this up sensibly?
Here is the complete list of things that rolreplication allows:
- create/drop replication slot
- pg_logical_slot_get_changes() and friends
- connect to walsender
For logical replication, we could slice it up this way:
- new user attribute allowing the creating of logical replication slots
- store owner of slot, allow drop and get based on ownership
- allow anyone to connect as walsender
Another problem is that the walsender command to create a replication
slot allows you to load an arbitrary plugin.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 22/03/17 03:38, Peter Eisentraut wrote:
On 3/20/17 15:10, Petr Jelinek wrote:
Hmm but REPLICATION role can do basebackup/consume wal, so how does
giving it limited publication access help? Wouldn't we need some
SUBSCRIPTION role/grant used instead for logical replication connections
instead of REPLICATION for this to make sense?Since we're splitting up the pg_hba.conf setup for logical and physical
connections, it would probably not matter.
Hmm yeah I know about that, I am not quite clear on how that change
affects this.
But just to think it through, how could we split this up sensibly?
Here is the complete list of things that rolreplication allows:
- create/drop replication slot
- pg_logical_slot_get_changes() and friends
- connect to walsenderFor logical replication, we could slice it up this way:
- new user attribute allowing the creating of logical replication slots
- store owner of slot, allow drop and get based on ownership
- allow anyone to connect as walsender
I am not quite sure we can do the owner part. Slots are not usual
catalog and there is this idea that it should be possible to create them
on standby (at least it was reason why our last year proposal to
propagate slot creation/updates via WAL was shot down). So we can't do
any of the dependency stuff for them.
Another problem is that the walsender command to create a replication
slot allows you to load an arbitrary plugin.
Yeah I am also not sure what to do with the SQL interface tbh as that
has same problem.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/18/17 09:31, Petr Jelinek wrote:
0001 Refine rules for altering publication owner
kind of a bug fix
Agreed, this can be committed as is.
0002 Change logical replication pg_hba.conf use
This was touched upon in the discussion at
</messages/by-id/CAB7nPqRf8eOv15SPQJbC1npJoDWTNPMTNp6AvMN-XWwB53h2Cg@mail.gmail.com>
and seems to have been viewed favorably there.Seems like a good idea and I think can be committed as well.
Committed these two.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/22/17 08:12, Petr Jelinek wrote:
On 22/03/17 03:38, Peter Eisentraut wrote:
On 3/20/17 15:10, Petr Jelinek wrote:
Hmm but REPLICATION role can do basebackup/consume wal, so how does
giving it limited publication access help? Wouldn't we need some
SUBSCRIPTION role/grant used instead for logical replication connections
instead of REPLICATION for this to make sense?Since we're splitting up the pg_hba.conf setup for logical and physical
connections, it would probably not matter.Hmm yeah I know about that, I am not quite clear on how that change
affects this.
Well, the explanation for what I had in mind is too complicated to even
put into words, so it's probably not a good idea. ;-)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/15/17 21:54, Peter Eisentraut wrote:
0001 Refine rules for altering publication owner
0002 Change logical replication pg_hba.conf use
These two were committed.
0003 Add USAGE privilege for publications
I'm withdrawing this one for now, because of some issues that were
discussed in the thread.
0004 Add subscription apply worker privilege checks
0005 Add CREATE SUBSCRIPTION privilege on databases
It would be nice to reach a conclusion on these (the second one
particularly), because otherwise we'll be stuck with only superusers
being allowed to create subscriptions.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut wrote:
On 3/15/17 21:54, Peter Eisentraut wrote:
0004 Add subscription apply worker privilege checks
0005 Add CREATE SUBSCRIPTION privilege on databasesIt would be nice to reach a conclusion on these (the second one
particularly), because otherwise we'll be stuck with only superusers
being allowed to create subscriptions.
I note that the CREATE privilege on databases, which previously only
enabled schema creation, now also allows to create publications. I
wonder what is different about subscriptions that we need a separate
CREATE SUBSCRIPTION privilege; could we allow the three things under the
same privilege type? (I suspect not; why give logical replication
controls to users who in previous releases were only able to create
schemas?) If not, does it make sense to have one privilege for both new
things, perhaps something like GRANT LOGICAL REPLICATION THINGIES? If
not, maybe we should have three separate priv bits: GRANT CREATE for
schemas, GRANT CREATE PUBLICATION and GRANT CREATE SUBSCRIPTION?
So this CREATE SUBSCRIPTION priv actually gives you the power to cause
the system to open network connections to the outside world. It's not
something you give freely to random strangers -- should be guarded
moderately tight, because it could be used as covert channel for data
leaking. However, it's 1000x better than requiring superuser for
subscription creation, so +1 for the current approach.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 29/03/17 20:55, Alvaro Herrera wrote:
Peter Eisentraut wrote:
On 3/15/17 21:54, Peter Eisentraut wrote:
0004 Add subscription apply worker privilege checks
0005 Add CREATE SUBSCRIPTION privilege on databasesIt would be nice to reach a conclusion on these (the second one
particularly), because otherwise we'll be stuck with only superusers
being allowed to create subscriptions.I note that the CREATE privilege on databases, which previously only
enabled schema creation, now also allows to create publications. I
wonder what is different about subscriptions that we need a separate
CREATE SUBSCRIPTION privilege; could we allow the three things under the
same privilege type? (I suspect not; why give logical replication
controls to users who in previous releases were only able to create
schemas?) If not, does it make sense to have one privilege for both new
things, perhaps something like GRANT LOGICAL REPLICATION THINGIES? If
not, maybe we should have three separate priv bits: GRANT CREATE for
schemas, GRANT CREATE PUBLICATION and GRANT CREATE SUBSCRIPTION?So this CREATE SUBSCRIPTION priv actually gives you the power to cause
the system to open network connections to the outside world. It's not
something you give freely to random strangers -- should be guarded
moderately tight, because it could be used as covert channel for data
leaking. However, it's 1000x better than requiring superuser for
subscription creation, so +1 for the current approach.
Plus on the other hand you might want to allow somebody to stream data
from another server but not necessarily allow said person to create new
objects in the database which standard CREATE privilege would allow. So
I think it makes sense to push this approach.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/29/17 19:01, Petr Jelinek wrote:
So this CREATE SUBSCRIPTION priv actually gives you the power to cause
the system to open network connections to the outside world. It's not
something you give freely to random strangers -- should be guarded
moderately tight, because it could be used as covert channel for data
leaking. However, it's 1000x better than requiring superuser for
subscription creation, so +1 for the current approach.Plus on the other hand you might want to allow somebody to stream data
from another server but not necessarily allow said person to create new
objects in the database which standard CREATE privilege would allow. So
I think it makes sense to push this approach.
One new concern I just thought about is that having GRANT whatever ON
DATABASE would allow a database owner to assign these privileges, but a
database owner is not necessarily someone highly privileged to make this
decision.
So I'm prepared to set this patch to returned with feedback for now.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers