adjust ACL owners for REASSIGN and ALTER OWNER TO
authorAlvaro Herrera
Mon, 21 Dec 2015 22:16:15 +0000 (19:16 -0300)
committerAlvaro Herrera
Mon, 21 Dec 2015 22:16:15 +0000 (19:16 -0300)
When REASSIGN and ALTER OWNER TO are used, both the object owner and ACL
list should be changed from the old owner to the new owner. This patch
fixes types, foreign data wrappers, and foreign servers to change their
ACL list properly;  they already changed owners properly.

Report by Alexey Bashtanov

This is a backpatch of commit 59367fdf97c (for bug #9923) by Bruce
Momjian to branches 9.1 - 9.4; it wasn't backpatched originally out of
concerns that it would create a backwards compatibility problem, but per
discussion related to bug #13666 that turns out to have been misguided.
(Therefore, the entry in the 9.5 release notes should be removed.)

Note that 9.1 didn't have privileges on types (which were introduced by
commit 729205571e81), so this commit only changes foreign-data related
objects in that branch.

Discussion: http://www.postgresql.org/message-id/20151216224004[email protected]
http://www.postgresql.org/message-id/10227.1450373793@sss.pgh.pa.us

src/backend/commands/foreigncmds.c
src/backend/commands/typecmds.c
src/test/regress/expected/foreign_data.out

index 596db1ad764cb5ac410a5de49f1946ad47366210..41e853d2d46b573013d29c2c3c0b6b274f318268 100644 (file)
@@ -213,6 +213,12 @@ static void
 AlterForeignDataWrapperOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
 {
    Form_pg_foreign_data_wrapper form;
+   Datum       repl_val[Natts_pg_foreign_data_wrapper];
+   bool        repl_null[Natts_pg_foreign_data_wrapper];
+   bool        repl_repl[Natts_pg_foreign_data_wrapper];
+   Acl        *newAcl;
+   Datum       aclDatum;
+   bool        isNull;
 
    form = (Form_pg_foreign_data_wrapper) GETSTRUCT(tup);
 
@@ -234,7 +240,27 @@ AlterForeignDataWrapperOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerI
 
    if (form->fdwowner != newOwnerId)
    {
-       form->fdwowner = newOwnerId;
+       memset(repl_null, false, sizeof(repl_null));
+       memset(repl_repl, false, sizeof(repl_repl));
+
+       repl_repl[Anum_pg_foreign_data_wrapper_fdwowner - 1] = true;
+       repl_val[Anum_pg_foreign_data_wrapper_fdwowner - 1] = ObjectIdGetDatum(newOwnerId);
+
+       aclDatum = heap_getattr(tup,
+                               Anum_pg_foreign_data_wrapper_fdwacl,
+                               RelationGetDescr(rel),
+                               &isNull);
+       /* Null ACLs do not require changes */
+       if (!isNull)
+       {
+           newAcl = aclnewowner(DatumGetAclP(aclDatum),
+                                form->fdwowner, newOwnerId);
+           repl_repl[Anum_pg_foreign_data_wrapper_fdwacl - 1] = true;
+           repl_val[Anum_pg_foreign_data_wrapper_fdwacl - 1] = PointerGetDatum(newAcl);
+       }
+
+       tup = heap_modify_tuple(tup, RelationGetDescr(rel), repl_val, repl_null,
+                               repl_repl);
 
        simple_heap_update(rel, &tup->t_self, tup);
        CatalogUpdateIndexes(rel, tup);
@@ -315,6 +341,12 @@ static void
 AlterForeignServerOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
 {
    Form_pg_foreign_server form;
+   Datum       repl_val[Natts_pg_foreign_server];
+   bool        repl_null[Natts_pg_foreign_server];
+   bool        repl_repl[Natts_pg_foreign_server];
+   Acl        *newAcl;
+   Datum       aclDatum;
+   bool        isNull;
 
    form = (Form_pg_foreign_server) GETSTRUCT(tup);
 
@@ -346,7 +378,27 @@ AlterForeignServerOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
            }
        }
 
-       form->srvowner = newOwnerId;
+       memset(repl_null, false, sizeof(repl_null));
+       memset(repl_repl, false, sizeof(repl_repl));
+
+       repl_repl[Anum_pg_foreign_server_srvowner - 1] = true;
+       repl_val[Anum_pg_foreign_server_srvowner - 1] = ObjectIdGetDatum(newOwnerId);
+
+       aclDatum = heap_getattr(tup,
+                               Anum_pg_foreign_server_srvacl,
+                               RelationGetDescr(rel),
+                               &isNull);
+       /* Null ACLs do not require changes */
+       if (!isNull)
+       {
+           newAcl = aclnewowner(DatumGetAclP(aclDatum),
+                                form->srvowner, newOwnerId);
+           repl_repl[Anum_pg_foreign_server_srvacl - 1] = true;
+           repl_val[Anum_pg_foreign_server_srvacl - 1] = PointerGetDatum(newAcl);
+       }
+
+       tup = heap_modify_tuple(tup, RelationGetDescr(rel), repl_val, repl_null,
+                               repl_repl);
 
        simple_heap_update(rel, &tup->t_self, tup);
        CatalogUpdateIndexes(rel, tup);
index b08cc2b6b9a80c7383bd8e30a1e085d3d28ccdc9..b25a7b28e694d186f8643cb88bd6f981ea11f823 100644 (file)
@@ -3316,12 +3316,34 @@ AlterTypeOwner(List *names, Oid newOwnerId, ObjectType objecttype)
            ATExecChangeOwner(typTup->typrelid, newOwnerId, true, AccessExclusiveLock);
        else
        {
-           /*
-            * We can just apply the modification directly.
-            *
-            * okay to scribble on typTup because it's a copy
-            */
-           typTup->typowner = newOwnerId;
+           Datum       repl_val[Natts_pg_type];
+           bool        repl_null[Natts_pg_type];
+           bool        repl_repl[Natts_pg_type];
+           Acl        *newAcl;
+           Datum       aclDatum;
+           bool        isNull;
+
+           memset(repl_null, false, sizeof(repl_null));
+           memset(repl_repl, false, sizeof(repl_repl));
+
+           repl_repl[Anum_pg_type_typowner - 1] = true;
+           repl_val[Anum_pg_type_typowner - 1] = ObjectIdGetDatum(newOwnerId);
+
+           aclDatum = heap_getattr(tup,
+                                   Anum_pg_type_typacl,
+                                   RelationGetDescr(rel),
+                                   &isNull);
+           /* Null ACLs do not require changes */
+           if (!isNull)
+           {
+               newAcl = aclnewowner(DatumGetAclP(aclDatum),
+                                    typTup->typowner, newOwnerId);
+               repl_repl[Anum_pg_type_typacl - 1] = true;
+               repl_val[Anum_pg_type_typacl - 1] = PointerGetDatum(newAcl);
+           }
+
+           tup = heap_modify_tuple(tup, RelationGetDescr(rel), repl_val, repl_null,
+                                   repl_repl);
 
            simple_heap_update(rel, &tup->t_self, tup);
 
@@ -3364,6 +3386,12 @@ AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId,
    Relation    rel;
    HeapTuple   tup;
    Form_pg_type typTup;
+   Datum       repl_val[Natts_pg_type];
+   bool        repl_null[Natts_pg_type];
+   bool        repl_repl[Natts_pg_type];
+   Acl        *newAcl;
+   Datum       aclDatum;
+   bool        isNull;
 
    rel = heap_open(TypeRelationId, RowExclusiveLock);
 
@@ -3372,10 +3400,27 @@ AlterTypeOwnerInternal(Oid typeOid, Oid newOwnerId,
        elog(ERROR, "cache lookup failed for type %u", typeOid);
    typTup = (Form_pg_type) GETSTRUCT(tup);
 
-   /*
-    * Modify the owner --- okay to scribble on typTup because it's a copy
-    */
-   typTup->typowner = newOwnerId;
+   memset(repl_null, false, sizeof(repl_null));
+   memset(repl_repl, false, sizeof(repl_repl));
+
+   repl_repl[Anum_pg_type_typowner - 1] = true;
+   repl_val[Anum_pg_type_typowner - 1] = ObjectIdGetDatum(newOwnerId);
+
+   aclDatum = heap_getattr(tup,
+                           Anum_pg_type_typacl,
+                           RelationGetDescr(rel),
+                           &isNull);
+   /* Null ACLs do not require changes */
+   if (!isNull)
+   {
+       newAcl = aclnewowner(DatumGetAclP(aclDatum),
+                            typTup->typowner, newOwnerId);
+       repl_repl[Anum_pg_type_typacl - 1] = true;
+       repl_val[Anum_pg_type_typacl - 1] = PointerGetDatum(newAcl);
+   }
+
+   tup = heap_modify_tuple(tup, RelationGetDescr(rel), repl_val, repl_null,
+                           repl_repl);
 
    simple_heap_update(rel, &tup->t_self, tup);
 
index bef0bd353b8d836132a12a73e0c496da22a38ae8..857f1cefb2dba4ab9a71787470331be16bd303e0 100644 (file)
@@ -422,40 +422,38 @@ ERROR:  role "regress_test_indirect" cannot be dropped because some objects depe
 DETAIL:  owner of server s1
 privileges for foreign-data wrapper foo
 \des+
-                                                                        List of foreign servers
- Name |         Owner         | Foreign-data wrapper |            Access privileges            |  Type  | Version |             FDW Options              | Description 
-------+-----------------------+----------------------+-----------------------------------------+--------+---------+--------------------------------------+-------------
- s1   | regress_test_indirect | foo                  | foreign_data_user=U/foreign_data_user  +|        | 1.1     | (servername 's1')                    | 
-      |                       |                      | regress_test_role=U/foreign_data_user   |        |         |                                      | 
- s2   | foreign_data_user     | foo                  |                                         |        | 1.1     | (host 'a', dbname 'b')               | 
- s3   | foreign_data_user     | foo                  |                                         | oracle |         | ("tns name" 'orcl', port '1521')     | 
- s4   | foreign_data_user     | foo                  |                                         | oracle |         | (host 'a', dbname 'b')               | 
- s5   | foreign_data_user     | foo                  |                                         |        | 15.0    |                                      | 
- s6   | foreign_data_user     | foo                  | foreign_data_user=U/foreign_data_user  +|        | 16.0    | (host 'a', dbname 'b')               | 
-      |                       |                      | regress_test_role2=U*/foreign_data_user |        |         |                                      | 
- s7   | foreign_data_user     | foo                  |                                         | oracle | 17.0    | (host 'a', dbname 'b')               | 
- s8   | foreign_data_user     | postgresql           |                                         |        |         | (dbname 'db1', connect_timeout '30') | 
- t1   | regress_test_role     | foo                  |                                         |        |         |                                      | 
- t2   | regress_test_role     | foo                  |                                         |        |         |                                      | 
+                                                                           List of foreign servers
+ Name |         Owner         | Foreign-data wrapper |               Access privileges               |  Type  | Version |             FDW Options              | Description 
+------+-----------------------+----------------------+-----------------------------------------------+--------+---------+--------------------------------------+-------------
+ s1   | regress_test_indirect | foo                  | regress_test_indirect=U/regress_test_indirect |        | 1.1     | (servername 's1')                    | 
+ s2   | foreign_data_user     | foo                  |                                               |        | 1.1     | (host 'a', dbname 'b')               | 
+ s3   | foreign_data_user     | foo                  |                                               | oracle |         | ("tns name" 'orcl', port '1521')     | 
+ s4   | foreign_data_user     | foo                  |                                               | oracle |         | (host 'a', dbname 'b')               | 
+ s5   | foreign_data_user     | foo                  |                                               |        | 15.0    |                                      | 
+ s6   | foreign_data_user     | foo                  | foreign_data_user=U/foreign_data_user        +|        | 16.0    | (host 'a', dbname 'b')               | 
+      |                       |                      | regress_test_role2=U*/foreign_data_user       |        |         |                                      | 
+ s7   | foreign_data_user     | foo                  |                                               | oracle | 17.0    | (host 'a', dbname 'b')               | 
+ s8   | foreign_data_user     | postgresql           |                                               |        |         | (dbname 'db1', connect_timeout '30') | 
+ t1   | regress_test_role     | foo                  |                                               |        |         |                                      | 
+ t2   | regress_test_role     | foo                  |                                               |        |         |                                      | 
 (10 rows)
 
 ALTER SERVER s8 RENAME to s8new;
 \des+
-                                                                        List of foreign servers
- Name  |         Owner         | Foreign-data wrapper |            Access privileges            |  Type  | Version |             FDW Options              | Description 
--------+-----------------------+----------------------+-----------------------------------------+--------+---------+--------------------------------------+-------------
- s1    | regress_test_indirect | foo                  | foreign_data_user=U/foreign_data_user  +|        | 1.1     | (servername 's1')                    | 
-       |                       |                      | regress_test_role=U/foreign_data_user   |        |         |                                      | 
- s2    | foreign_data_user     | foo                  |                                         |        | 1.1     | (host 'a', dbname 'b')               | 
- s3    | foreign_data_user     | foo                  |                                         | oracle |         | ("tns name" 'orcl', port '1521')     | 
- s4    | foreign_data_user     | foo                  |                                         | oracle |         | (host 'a', dbname 'b')               | 
- s5    | foreign_data_user     | foo                  |                                         |        | 15.0    |                                      | 
- s6    | foreign_data_user     | foo                  | foreign_data_user=U/foreign_data_user  +|        | 16.0    | (host 'a', dbname 'b')               | 
-       |                       |                      | regress_test_role2=U*/foreign_data_user |        |         |                                      | 
- s7    | foreign_data_user     | foo                  |                                         | oracle | 17.0    | (host 'a', dbname 'b')               | 
- s8new | foreign_data_user     | postgresql           |                                         |        |         | (dbname 'db1', connect_timeout '30') | 
- t1    | regress_test_role     | foo                  |                                         |        |         |                                      | 
- t2    | regress_test_role     | foo                  |                                         |        |         |                                      | 
+                                                                           List of foreign servers
+ Name  |         Owner         | Foreign-data wrapper |               Access privileges               |  Type  | Version |             FDW Options              | Description 
+-------+-----------------------+----------------------+-----------------------------------------------+--------+---------+--------------------------------------+-------------
+ s1    | regress_test_indirect | foo                  | regress_test_indirect=U/regress_test_indirect |        | 1.1     | (servername 's1')                    | 
+ s2    | foreign_data_user     | foo                  |                                               |        | 1.1     | (host 'a', dbname 'b')               | 
+ s3    | foreign_data_user     | foo                  |                                               | oracle |         | ("tns name" 'orcl', port '1521')     | 
+ s4    | foreign_data_user     | foo                  |                                               | oracle |         | (host 'a', dbname 'b')               | 
+ s5    | foreign_data_user     | foo                  |                                               |        | 15.0    |                                      | 
+ s6    | foreign_data_user     | foo                  | foreign_data_user=U/foreign_data_user        +|        | 16.0    | (host 'a', dbname 'b')               | 
+       |                       |                      | regress_test_role2=U*/foreign_data_user       |        |         |                                      | 
+ s7    | foreign_data_user     | foo                  |                                               | oracle | 17.0    | (host 'a', dbname 'b')               | 
+ s8new | foreign_data_user     | postgresql           |                                               |        |         | (dbname 'db1', connect_timeout '30') | 
+ t1    | regress_test_role     | foo                  |                                               |        |         |                                      | 
+ t2    | regress_test_role     | foo                  |                                               |        |         |                                      | 
 (10 rows)
 
 ALTER SERVER s8new RENAME to s8;
@@ -898,21 +896,21 @@ SELECT * FROM information_schema.user_mapping_options ORDER BY lower(authorizati
 (7 rows)
 
 SELECT * FROM information_schema.usage_privileges WHERE object_type LIKE 'FOREIGN%' AND object_name IN ('s6', 'foo') ORDER BY 1, 2, 3, 4, 5;
-      grantor      |        grantee        | object_catalog | object_schema | object_name |     object_type      | privilege_type | is_grantable 
--------------------+-----------------------+----------------+---------------+-------------+----------------------+----------------+--------------
- foreign_data_user | foreign_data_user     | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | YES
- foreign_data_user | foreign_data_user     | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
- foreign_data_user | regress_test_indirect | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | NO
foreign_data_user | regress_test_role2    | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
+        grantor        |        grantee        | object_catalog | object_schema | object_name |     object_type      | privilege_type | is_grantable 
+-----------------------+-----------------------+----------------+---------------+-------------+----------------------+----------------+--------------
+ foreign_data_user     | foreign_data_user     | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | YES
+ foreign_data_user     | regress_test_indirect | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | NO
+ regress_test_indirect | regress_test_indirect | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
regress_test_indirect | regress_test_role2    | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
 (4 rows)
 
 SELECT * FROM information_schema.role_usage_grants WHERE object_type LIKE 'FOREIGN%' AND object_name IN ('s6', 'foo') ORDER BY 1, 2, 3, 4, 5;
-      grantor      |        grantee        | object_catalog | object_schema | object_name |     object_type      | privilege_type | is_grantable 
--------------------+-----------------------+----------------+---------------+-------------+----------------------+----------------+--------------
- foreign_data_user | foreign_data_user     | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | YES
- foreign_data_user | foreign_data_user     | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
- foreign_data_user | regress_test_indirect | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | NO
foreign_data_user | regress_test_role2    | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
+        grantor        |        grantee        | object_catalog | object_schema | object_name |     object_type      | privilege_type | is_grantable 
+-----------------------+-----------------------+----------------+---------------+-------------+----------------------+----------------+--------------
+ foreign_data_user     | foreign_data_user     | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | YES
+ foreign_data_user     | regress_test_indirect | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | NO
+ regress_test_indirect | regress_test_indirect | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
regress_test_indirect | regress_test_role2    | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
 (4 rows)
 
 SELECT * FROM information_schema.foreign_tables ORDER BY 1, 2, 3;
@@ -941,18 +939,20 @@ SELECT * FROM information_schema.user_mapping_options ORDER BY 1, 2, 3, 4;
 (5 rows)
 
 SELECT * FROM information_schema.usage_privileges WHERE object_type LIKE 'FOREIGN%' AND object_name IN ('s6', 'foo') ORDER BY 1, 2, 3, 4, 5;
-      grantor      |        grantee        | object_catalog | object_schema | object_name |     object_type      | privilege_type | is_grantable 
--------------------+-----------------------+----------------+---------------+-------------+----------------------+----------------+--------------
- foreign_data_user | regress_test_indirect | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | NO
- foreign_data_user | regress_test_role2    | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
-(2 rows)
+        grantor        |        grantee        | object_catalog | object_schema | object_name |     object_type      | privilege_type | is_grantable 
+-----------------------+-----------------------+----------------+---------------+-------------+----------------------+----------------+--------------
+ foreign_data_user     | regress_test_indirect | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | NO
+ regress_test_indirect | regress_test_indirect | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
+ regress_test_indirect | regress_test_role2    | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
+(3 rows)
 
 SELECT * FROM information_schema.role_usage_grants WHERE object_type LIKE 'FOREIGN%' AND object_name IN ('s6', 'foo') ORDER BY 1, 2, 3, 4, 5;
-      grantor      |        grantee        | object_catalog | object_schema | object_name |     object_type      | privilege_type | is_grantable 
--------------------+-----------------------+----------------+---------------+-------------+----------------------+----------------+--------------
- foreign_data_user | regress_test_indirect | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | NO
- foreign_data_user | regress_test_role2    | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
-(2 rows)
+        grantor        |        grantee        | object_catalog | object_schema | object_name |     object_type      | privilege_type | is_grantable 
+-----------------------+-----------------------+----------------+---------------+-------------+----------------------+----------------+--------------
+ foreign_data_user     | regress_test_indirect | regression     |               | foo         | FOREIGN DATA WRAPPER | USAGE          | NO
+ regress_test_indirect | regress_test_indirect | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
+ regress_test_indirect | regress_test_role2    | regression     |               | s6          | FOREIGN SERVER       | USAGE          | YES
+(3 rows)
 
 DROP USER MAPPING FOR current_user SERVER t1;
 SET ROLE regress_test_role2;