Handle ALTER EXTENSION ADD/DROP with pg_init_privs

In commit 6c268df1, pg_init_privs was added to track the initial
privileges of catalog objects and extensions.  Unfortunately, that
commit didn't include understanding of ALTER EXTENSION ADD/DROP, which
allows the objects associated with an extension to be changed after the
initial CREATE EXTENSION script has been run.

The result of this meant that ACLs for objects added through
ALTER EXTENSION ADD were not recorded into pg_init_privs and we would
end up including those ACLs in pg_dump when we shouldn't have.

This commit corrects that by making sure to have pg_init_privs updated
when ALTER EXTENSION ADD/DROP is run, recording the permissions as they
are at ALTER EXTENSION ADD time, and removing any if/when ALTER
EXTENSION DROP is called.

This issue was pointed out by Moshe Jacobson as commentary on bug #14456
(which was actually a bug about versions prior to 9.6 not handling
custom ACLs on extensions correctly, an issue now addressed with
pg_init_privs in 9.6).

Back-patch to 9.6 where pg_init_privs was introduced.

src/backend/commands/extension.c

@@ -52,6 +52,7 @@
 #include "nodes/makefuncs.h"
 #include "storage/fd.h"
 #include "tcop/utility.h"
+#include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
@@ -3240,6 +3241,16 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
 		 * OK, add the dependency.
 		 */
 		recordDependencyOn(&object, &extension, DEPENDENCY_EXTENSION);
+
+		/*
+		 * Also record the initial ACL on the object, if any.
+		 *
+		 * Note that this will handle the object's ACLs, as well as any ACLs
+		 * on object subIds.  (In other words, when the object is a table,
+		 * this will record the table's ACL and the ACLs for the columns on
+		 * the table, if any).
+		 */
+		recordExtObjInitPriv(object.objectId, object.classId);
 	}
 	else
 	{
@@ -3267,6 +3278,16 @@ ExecAlterExtensionContentsStmt(AlterExtensionContentsStmt *stmt,
 		 */
 		if (object.classId == RelationRelationId)
 			extension_config_remove(extension.objectId, object.objectId);
+
+		/*
+		 * Remove all the initial ACLs, if any.
+		 *
+		 * Note that this will remove the object's ACLs, as well as any ACLs
+		 * on object subIds.  (In other words, when the object is a table,
+		 * this will remove the table's ACL and the ACLs for the columns on
+		 * the table, if any).
+		 */
+		removeExtObjInitPriv(object.objectId, object.classId);
 	}
 
 	InvokeObjectPostAlterHook(ExtensionRelationId, extension.objectId, 0);

src/include/utils/acl.h

@@ -300,6 +300,10 @@ extern void aclcheck_error_col(AclResult aclerr, AclObjectKind objectkind,
 
 extern void aclcheck_error_type(AclResult aclerr, Oid typeOid);
 
+extern void recordExtObjInitPriv(Oid objoid, Oid classoid);
+extern void removeExtObjInitPriv(Oid objoid, Oid classoid);
+
+
 /* ownercheck routines just return true (owner) or false (not) */
 extern bool pg_class_ownercheck(Oid class_oid, Oid roleid);
 extern bool pg_type_ownercheck(Oid type_oid, Oid roleid);

src/test/modules/test_pg_dump/expected/test_pg_dump.out

-SELECT 1;
- ?column? 
-----------
-        1
-(1 row)
-
+CREATE ROLE regress_dump_test_role;
+CREATE EXTENSION test_pg_dump;
+ALTER EXTENSION test_pg_dump ADD DATABASE postgres; -- error
+ERROR:  syntax error at or near "DATABASE"
+LINE 1: ALTER EXTENSION test_pg_dump ADD DATABASE postgres;
+                                         ^
+CREATE TABLE test_pg_dump_t1 (c1 int);
+CREATE VIEW test_pg_dump_v1 AS SELECT * FROM test_pg_dump_t1;
+CREATE MATERIALIZED VIEW test_pg_dump_mv1 AS SELECT * FROM test_pg_dump_t1;
+CREATE SCHEMA test_pg_dump_s1;
+CREATE TYPE test_pg_dump_e1 AS ENUM ('abc', 'def');
+CREATE AGGREGATE newavg (
+   sfunc = int4_avg_accum, basetype = int4, stype = _int8,
+   finalfunc = int8_avg,
+   initcond1 = '{0,0}'
+);
+CREATE FUNCTION test_pg_dump(int) RETURNS int AS $$
+BEGIN
+RETURN abs($1);
+END
+$$ LANGUAGE plpgsql IMMUTABLE;
+CREATE OPERATOR ==== (
+    LEFTARG = int,
+    RIGHTARG = int,
+    PROCEDURE = int4eq,
+    COMMUTATOR = ====
+);
+CREATE ACCESS METHOD gist2 TYPE INDEX HANDLER gisthandler;
+CREATE TYPE casttesttype;
+CREATE FUNCTION casttesttype_in(cstring)
+   RETURNS casttesttype
+   AS 'textin'
+   LANGUAGE internal STRICT IMMUTABLE;
+NOTICE:  return type casttesttype is only a shell
+CREATE FUNCTION casttesttype_out(casttesttype)
+   RETURNS cstring
+   AS 'textout'
+   LANGUAGE internal STRICT IMMUTABLE;
+NOTICE:  argument type casttesttype is only a shell
+CREATE TYPE casttesttype (
+   internallength = variable,
+   input = casttesttype_in,
+   output = casttesttype_out,
+   alignment = int4
+);
+CREATE CAST (text AS casttesttype) WITHOUT FUNCTION;
+CREATE FOREIGN DATA WRAPPER dummy;
+CREATE SERVER s0 FOREIGN DATA WRAPPER dummy;
+CREATE FOREIGN TABLE ft1 (
+        c1 integer OPTIONS ("param 1" 'val1') NOT NULL,
+        c2 text OPTIONS (param2 'val2', param3 'val3') CHECK (c2 <> ''),
+        c3 date,
+        CHECK (c3 BETWEEN '1994-01-01'::date AND '1994-01-31'::date)
+) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+REVOKE EXECUTE ON FUNCTION test_pg_dump(int) FROM PUBLIC;
+GRANT EXECUTE ON FUNCTION test_pg_dump(int) TO regress_dump_test_role;
+GRANT SELECT (c1) ON test_pg_dump_t1 TO regress_dump_test_role;
+GRANT SELECT ON test_pg_dump_v1 TO regress_dump_test_role;
+GRANT USAGE ON FOREIGN DATA WRAPPER dummy TO regress_dump_test_role;
+GRANT USAGE ON FOREIGN SERVER s0 TO regress_dump_test_role;
+GRANT SELECT (c1) ON ft1 TO regress_dump_test_role;
+GRANT SELECT ON ft1 TO regress_dump_test_role;
+GRANT UPDATE ON test_pg_dump_mv1 TO regress_dump_test_role;
+GRANT USAGE ON SCHEMA test_pg_dump_s1 TO regress_dump_test_role;
+GRANT USAGE ON TYPE test_pg_dump_e1 TO regress_dump_test_role;
+ALTER EXTENSION test_pg_dump ADD ACCESS METHOD gist2;
+ALTER EXTENSION test_pg_dump ADD AGGREGATE newavg(int4);
+ALTER EXTENSION test_pg_dump ADD CAST (text AS casttesttype);
+ALTER EXTENSION test_pg_dump ADD FOREIGN DATA WRAPPER dummy;
+ALTER EXTENSION test_pg_dump ADD FOREIGN TABLE ft1;
+ALTER EXTENSION test_pg_dump ADD MATERIALIZED VIEW test_pg_dump_mv1;
+ALTER EXTENSION test_pg_dump ADD OPERATOR ==== (int, int);
+ALTER EXTENSION test_pg_dump ADD SCHEMA test_pg_dump_s1;
+ALTER EXTENSION test_pg_dump ADD SERVER s0;
+ALTER EXTENSION test_pg_dump ADD FUNCTION test_pg_dump(int);
+ALTER EXTENSION test_pg_dump ADD TABLE test_pg_dump_t1;
+ALTER EXTENSION test_pg_dump ADD TYPE test_pg_dump_e1;
+ALTER EXTENSION test_pg_dump ADD VIEW test_pg_dump_v1;
+REVOKE SELECT (c1) ON test_pg_dump_t1 FROM regress_dump_test_role;
+REVOKE SELECT ON test_pg_dump_v1 FROM regress_dump_test_role;
+REVOKE USAGE ON FOREIGN DATA WRAPPER dummy FROM regress_dump_test_role;
+ALTER EXTENSION test_pg_dump DROP ACCESS METHOD gist2;
+ALTER EXTENSION test_pg_dump DROP AGGREGATE newavg(int4);
+ALTER EXTENSION test_pg_dump DROP CAST (text AS casttesttype);
+ALTER EXTENSION test_pg_dump DROP FOREIGN DATA WRAPPER dummy;
+ALTER EXTENSION test_pg_dump DROP FOREIGN TABLE ft1;
+ALTER EXTENSION test_pg_dump DROP FUNCTION test_pg_dump(int);
+ALTER EXTENSION test_pg_dump DROP MATERIALIZED VIEW test_pg_dump_mv1;
+ALTER EXTENSION test_pg_dump DROP OPERATOR ==== (int, int);
+ALTER EXTENSION test_pg_dump DROP SCHEMA test_pg_dump_s1;
+ALTER EXTENSION test_pg_dump DROP SERVER s0;
+ALTER EXTENSION test_pg_dump DROP TABLE test_pg_dump_t1;
+ALTER EXTENSION test_pg_dump DROP TYPE test_pg_dump_e1;
+ALTER EXTENSION test_pg_dump DROP VIEW test_pg_dump_v1;

src/test/modules/test_pg_dump/sql/test_pg_dump.sql

-SELECT 1;
+CREATE ROLE regress_dump_test_role;
+CREATE EXTENSION test_pg_dump;
+
+ALTER EXTENSION test_pg_dump ADD DATABASE postgres; -- error
+
+CREATE TABLE test_pg_dump_t1 (c1 int);
+CREATE VIEW test_pg_dump_v1 AS SELECT * FROM test_pg_dump_t1;
+CREATE MATERIALIZED VIEW test_pg_dump_mv1 AS SELECT * FROM test_pg_dump_t1;
+CREATE SCHEMA test_pg_dump_s1;
+CREATE TYPE test_pg_dump_e1 AS ENUM ('abc', 'def');
+
+CREATE AGGREGATE newavg (
+   sfunc = int4_avg_accum, basetype = int4, stype = _int8,
+   finalfunc = int8_avg,
+   initcond1 = '{0,0}'
+);
+
+CREATE FUNCTION test_pg_dump(int) RETURNS int AS $$
+BEGIN
+RETURN abs($1);
+END
+$$ LANGUAGE plpgsql IMMUTABLE;
+
+CREATE OPERATOR ==== (
+    LEFTARG = int,
+    RIGHTARG = int,
+    PROCEDURE = int4eq,
+    COMMUTATOR = ====
+);
+
+CREATE ACCESS METHOD gist2 TYPE INDEX HANDLER gisthandler;
+
+CREATE TYPE casttesttype;
+
+CREATE FUNCTION casttesttype_in(cstring)
+   RETURNS casttesttype
+   AS 'textin'
+   LANGUAGE internal STRICT IMMUTABLE;
+CREATE FUNCTION casttesttype_out(casttesttype)
+   RETURNS cstring
+   AS 'textout'
+   LANGUAGE internal STRICT IMMUTABLE;
+
+CREATE TYPE casttesttype (
+   internallength = variable,
+   input = casttesttype_in,
+   output = casttesttype_out,
+   alignment = int4
+);
+
+CREATE CAST (text AS casttesttype) WITHOUT FUNCTION;
+
+CREATE FOREIGN DATA WRAPPER dummy;
+
+CREATE SERVER s0 FOREIGN DATA WRAPPER dummy;
+
+CREATE FOREIGN TABLE ft1 (
+        c1 integer OPTIONS ("param 1" 'val1') NOT NULL,
+        c2 text OPTIONS (param2 'val2', param3 'val3') CHECK (c2 <> ''),
+        c3 date,
+        CHECK (c3 BETWEEN '1994-01-01'::date AND '1994-01-31'::date)
+) SERVER s0 OPTIONS (delimiter ',', quote '"', "be quoted" 'value');
+
+REVOKE EXECUTE ON FUNCTION test_pg_dump(int) FROM PUBLIC;
+GRANT EXECUTE ON FUNCTION test_pg_dump(int) TO regress_dump_test_role;
+
+GRANT SELECT (c1) ON test_pg_dump_t1 TO regress_dump_test_role;
+GRANT SELECT ON test_pg_dump_v1 TO regress_dump_test_role;
+GRANT USAGE ON FOREIGN DATA WRAPPER dummy TO regress_dump_test_role;
+GRANT USAGE ON FOREIGN SERVER s0 TO regress_dump_test_role;
+GRANT SELECT (c1) ON ft1 TO regress_dump_test_role;
+GRANT SELECT ON ft1 TO regress_dump_test_role;
+GRANT UPDATE ON test_pg_dump_mv1 TO regress_dump_test_role;
+GRANT USAGE ON SCHEMA test_pg_dump_s1 TO regress_dump_test_role;
+GRANT USAGE ON TYPE test_pg_dump_e1 TO regress_dump_test_role;
+
+ALTER EXTENSION test_pg_dump ADD ACCESS METHOD gist2;
+ALTER EXTENSION test_pg_dump ADD AGGREGATE newavg(int4);
+ALTER EXTENSION test_pg_dump ADD CAST (text AS casttesttype);
+ALTER EXTENSION test_pg_dump ADD FOREIGN DATA WRAPPER dummy;
+ALTER EXTENSION test_pg_dump ADD FOREIGN TABLE ft1;
+ALTER EXTENSION test_pg_dump ADD MATERIALIZED VIEW test_pg_dump_mv1;
+ALTER EXTENSION test_pg_dump ADD OPERATOR ==== (int, int);
+ALTER EXTENSION test_pg_dump ADD SCHEMA test_pg_dump_s1;
+ALTER EXTENSION test_pg_dump ADD SERVER s0;
+ALTER EXTENSION test_pg_dump ADD FUNCTION test_pg_dump(int);
+ALTER EXTENSION test_pg_dump ADD TABLE test_pg_dump_t1;
+ALTER EXTENSION test_pg_dump ADD TYPE test_pg_dump_e1;
+ALTER EXTENSION test_pg_dump ADD VIEW test_pg_dump_v1;
+
+REVOKE SELECT (c1) ON test_pg_dump_t1 FROM regress_dump_test_role;
+REVOKE SELECT ON test_pg_dump_v1 FROM regress_dump_test_role;
+REVOKE USAGE ON FOREIGN DATA WRAPPER dummy FROM regress_dump_test_role;
+
+ALTER EXTENSION test_pg_dump DROP ACCESS METHOD gist2;
+ALTER EXTENSION test_pg_dump DROP AGGREGATE newavg(int4);
+ALTER EXTENSION test_pg_dump DROP CAST (text AS casttesttype);
+ALTER EXTENSION test_pg_dump DROP FOREIGN DATA WRAPPER dummy;
+ALTER EXTENSION test_pg_dump DROP FOREIGN TABLE ft1;
+ALTER EXTENSION test_pg_dump DROP FUNCTION test_pg_dump(int);
+ALTER EXTENSION test_pg_dump DROP MATERIALIZED VIEW test_pg_dump_mv1;
+ALTER EXTENSION test_pg_dump DROP OPERATOR ==== (int, int);
+ALTER EXTENSION test_pg_dump DROP SCHEMA test_pg_dump_s1;
+ALTER EXTENSION test_pg_dump DROP SERVER s0;
+ALTER EXTENSION test_pg_dump DROP TABLE test_pg_dump_t1;
+ALTER EXTENSION test_pg_dump DROP TYPE test_pg_dump_e1;
+ALTER EXTENSION test_pg_dump DROP VIEW test_pg_dump_v1;

src/test/modules/test_pg_dump/t/001_base.pl

@@ -236,6 +236,30 @@ my %pgdump_runs = (
 # as the regexps are used for each run the test applies to.
 
 my %tests = (
+	'ALTER EXTENSION test_pg_dump' => {
+		create_order => 9,
+		create_sql   => 'ALTER EXTENSION test_pg_dump ADD TABLE regress_pg_dump_table_added;',
+		regexp => qr/^
+			\QCREATE TABLE regress_pg_dump_table_added (\E
+			\n\s+\Qcol1 integer NOT NULL,\E
+			\n\s+\Qcol2 integer\E
+			\n\);\n/xm,
+		like   => {
+			binary_upgrade      => 1,
+		},
+		unlike => {
+			clean               => 1,
+			clean_if_exists     => 1,
+			createdb            => 1,
+			defaults            => 1,
+			no_privs            => 1,
+			no_owner            => 1,
+			pg_dumpall_globals  => 1,
+			schema_only         => 1,
+			section_pre_data    => 1,
+			section_post_data   => 1,
+		 }, },
+
 	'CREATE EXTENSION test_pg_dump' => {
 		create_order => 2,
 		create_sql   => 'CREATE EXTENSION test_pg_dump;',
@@ -303,6 +327,30 @@ my %tests = (
 			section_post_data   => 1,
 		}, },
 
+	'CREATE TABLE regress_pg_dump_table_added' => {
+		create_order => 7,
+		create_sql => 'CREATE TABLE regress_pg_dump_table_added (col1 int not null, col2 int);',
+		regexp => qr/^
+			\QCREATE TABLE regress_pg_dump_table_added (\E
+			\n\s+\Qcol1 integer NOT NULL,\E
+			\n\s+\Qcol2 integer\E
+			\n\);\n/xm,
+		like   => {
+			binary_upgrade      => 1,
+		},
+		unlike => {
+			clean               => 1,
+			clean_if_exists     => 1,
+			createdb            => 1,
+			defaults            => 1,
+			no_privs            => 1,
+			no_owner            => 1,
+			pg_dumpall_globals  => 1,
+			schema_only         => 1,
+			section_pre_data    => 1,
+			section_post_data   => 1,
+		}, },
+
 	'CREATE SEQUENCE regress_pg_dump_seq' => {
 		regexp => qr/^
                     \QCREATE SEQUENCE regress_pg_dump_seq\E
@@ -413,6 +461,50 @@ my %tests = (
 			section_post_data   => 1,
 		}, },
 
+	'GRANT SELECT regress_pg_dump_table_added pre-ALTER EXTENSION' => {
+		create_order => 8,
+		create_sql   => 'GRANT SELECT ON regress_pg_dump_table_added TO regress_dump_test_role;',
+		regexp => qr/^
+			\QGRANT SELECT ON TABLE regress_pg_dump_table_added TO regress_dump_test_role;\E
+			\n/xm,
+		like   => {
+			binary_upgrade => 1,
+		},
+		unlike => {
+			clean              => 1,
+			clean_if_exists    => 1,
+			createdb           => 1,
+			defaults           => 1,
+			no_privs           => 1,
+			no_owner           => 1,
+			pg_dumpall_globals => 1,
+			schema_only        => 1,
+			section_pre_data   => 1,
+			section_post_data  => 1,
+		}, },
+
+	'REVOKE SELECT regress_pg_dump_table_added post-ALTER EXTENSION' => {
+		create_order => 10,
+		create_sql   => 'REVOKE SELECT ON regress_pg_dump_table_added FROM regress_dump_test_role;',
+		regexp => qr/^
+			\QREVOKE SELECT ON TABLE regress_pg_dump_table_added FROM regress_dump_test_role;\E
+			\n/xm,
+		like => {
+			binary_upgrade     => 1,
+			clean              => 1,
+			clean_if_exists    => 1,
+			createdb           => 1,
+			defaults           => 1,
+			no_owner           => 1,
+			schema_only        => 1,
+			section_pre_data   => 1,
+		},
+		unlike   => {
+			no_privs           => 1,
+			pg_dumpall_globals => 1,
+			section_post_data  => 1,
+		}, },
+
 	'GRANT SELECT ON TABLE regress_pg_dump_table' => {
 		regexp => qr/^
 			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\E\n