Allow to run pg_repack by non-superuser (#431)

The option --no-superuser-check allows to by-pass the check if the user is a superuser. That was done for users which run pg_repack on Amazon, where users cannot run it as a superuser.
The problem is that the option --no-superuser-check works only on the CLI level, skipping the check only by the pg_repack client. But there are also checks done by the extension functions exported to SQL.

The PR removes the check that the user is a superuser from functions repack_trigger() and repack_apply(). That check is redundant since queries executed by that functions can be executed by a user manually. Moreover repack_trigger() is a SECURITY DEFINER function, which means that it is executed with superuser privileges (pg_repack extension can be created only by superuser).

The PR changes privilege check in functions repack_swap(), repack_drop() and repack_index_swap(). Now that functions can be run by an owner of a table. That check is necessary since _swap functions swap relfilenodes on pg_class system catalog table. repack_drop() acquires ACCESS EXCLUSIVE lock and therefore it also requires privilege check.

META.json

@@ -2,7 +2,7 @@
    "name": "pg_repack",
    "abstract": "PostgreSQL module for data reorganization",
    "description": "Reorganize tables in PostgreSQL databases with minimal locks",
-   "version": "1.5.1",
+   "version": "1.5.2",
    "maintainer": [
        "Beena Emerson <memissemerson@gmail.com>",
        "Josh Kupershmidt <schmiddy@gmail.com>",
@@ -17,7 +17,7 @@
    "provides": {
       "pg_repack": {
          "file": "lib/pg_repack.sql",
-         "version": "1.5.1",
+         "version": "1.5.2",
          "abstract": "Reorganize tables in PostgreSQL databases with minimal locks"
       }
    },

bin/pgut/pgut.c

@@ -1235,9 +1235,12 @@ call_atexit_callbacks(bool fatal)
 {
 	pgut_atexit_item  *item;
 
-	for (item = pgut_atexit_stack; item; item = item->next)
+	item = pgut_atexit_stack;
+	while (item != NULL)
 	{
+		pgut_atexit_stack = pgut_atexit_stack->next;
 		item->callback(fatal, item->userdata);
+		item = pgut_atexit_stack;
 	}
 }
 

doc/pg_repack.rst

@@ -24,7 +24,8 @@ You can choose one of the following methods to reorganize:
 
 NOTICE:
 
-* Only superusers can use the utility.
+* Only superusers or owners of tables and indexes can use the utility. To run
+  pg_repack as an owner you need to use the option `--no-superuser-check`.
 * Target table must have a PRIMARY KEY, or at least a UNIQUE total index on a
   NOT NULL column.
 

doc/pg_repack_jp.rst

@@ -41,7 +41,8 @@ pg_repackでは再編成する方法として次のものが選択できます
 
 .. NOTICE:
   
-  * Only superusers can use the utility.
+  * Only superusers or owners of tables and indexes can use the utility. To run
+    pg_repack as an owner you need to use the option `--no-superuser-check`.
   * Target table must have a PRIMARY KEY, or at least a UNIQUE total index on a
     NOT NULL column.
 

lib/repack.c

@@ -102,10 +102,20 @@ static void swap_heap_or_index_files(Oid r1, Oid r2);
 
 /* check access authority */
 static void
-must_be_superuser(const char *func)
+must_be_owner(Oid relId)
 {
-	if (!superuser())
-		elog(ERROR, "must be superuser to use %s function", func);
+#if PG_VERSION_NUM >= 160000
+	if (!object_ownercheck(RelationRelationId, relId, GetUserId()))
+		aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relId)),
+					   get_rel_name(relId));
+#elif PG_VERSION_NUM >= 110000
+	if (!pg_class_ownercheck(relId, GetUserId()))
+		aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relId)),
+					   get_rel_name(relId));
+#else
+	if (!pg_class_ownercheck(relId, GetUserId()))
+		aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, get_rel_name(relId));
+#endif
 }
 
 
@@ -163,9 +173,6 @@ repack_trigger(PG_FUNCTION_ARGS)
 	Oid				relid;
 	StringInfo		sql;
 
-	/* authority check */
-	must_be_superuser("repack_trigger");
-
 	/* make sure it's called as a trigger at all */
 	if (!CALLED_AS_TRIGGER(fcinfo) ||
 		!TRIGGER_FIRED_AFTER(trigdata->tg_event) ||
@@ -259,9 +266,6 @@ repack_apply(PG_FUNCTION_ARGS)
 
 	initStringInfo(&sql_pop);
 
-	/* authority check */
-	must_be_superuser("repack_apply");
-
 	/* connect to SPI manager */
 	repack_init();
 
@@ -859,7 +863,7 @@ repack_swap(PG_FUNCTION_ARGS)
 	Oid				owner2;
 
 	/* authority check */
-	must_be_superuser("repack_swap");
+	must_be_owner(oid);
 
 	/* connect to SPI manager */
 	repack_init();
@@ -1060,7 +1064,7 @@ repack_drop(PG_FUNCTION_ARGS)
 	}
 
 	/* authority check */
-	must_be_superuser("repack_drop");
+	must_be_owner(oid);
 
 	/* connect to SPI manager */
 	repack_init();
@@ -1413,7 +1417,7 @@ repack_index_swap(PG_FUNCTION_ARGS)
 	HeapTuple          tuple;
 
 	/* authority check */
-	must_be_superuser("repack_index_swap");
+	must_be_owner(orig_idx_oid);
 
 	/* connect to SPI manager */
 	repack_init();

regress/expected/nosuper.out

@@ -16,4 +16,17 @@ ERROR: pg_repack failed with error: You must be a superuser to use pg_repack
 ERROR: pg_repack failed with error: ERROR:  permission denied for schema repack
 LINE 1: select repack.version(), repack.version_sql()
                ^
+GRANT ALL ON ALL TABLES IN SCHEMA repack TO nosuper;
+GRANT USAGE ON SCHEMA repack TO nosuper;
+-- => ERROR
+\! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check
+INFO: repacking table "public.tbl_cluster"
+ERROR: query failed: ERROR:  current transaction is aborted, commands ignored until end of transaction block
+DETAIL: query was: RESET lock_timeout
+ERROR: query failed: ERROR:  current transaction is aborted, commands ignored until end of transaction block
+DETAIL: query was: RESET lock_timeout
+ERROR:  permission denied for table tbl_cluster
+ERROR:  permission denied for table tbl_cluster
+REVOKE ALL ON ALL TABLES IN SCHEMA repack FROM nosuper;
+REVOKE USAGE ON SCHEMA repack FROM nosuper;
 DROP ROLE IF EXISTS nosuper;

regress/expected/nosuper_1.out

@@ -14,4 +14,19 @@ ERROR: pg_repack failed with error: You must be a superuser to use pg_repack
 -- => ERROR
 \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check
 ERROR: pg_repack failed with error: ERROR:  permission denied for schema repack
+LINE 1: select repack.version(), repack.version_sql()
+               ^
+GRANT ALL ON ALL TABLES IN SCHEMA repack TO nosuper;
+GRANT USAGE ON SCHEMA repack TO nosuper;
+-- => ERROR
+\! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check
+INFO: repacking table "public.tbl_cluster"
+ERROR: query failed: ERROR:  current transaction is aborted, commands ignored until end of transaction block
+DETAIL: query was: RESET lock_timeout
+ERROR: query failed: ERROR:  current transaction is aborted, commands ignored until end of transaction block
+DETAIL: query was: RESET lock_timeout
+ERROR:  permission denied for relation tbl_cluster
+ERROR:  permission denied for relation tbl_cluster
+REVOKE ALL ON ALL TABLES IN SCHEMA repack FROM nosuper;
+REVOKE USAGE ON SCHEMA repack FROM nosuper;
 DROP ROLE IF EXISTS nosuper;

regress/sql/nosuper.sql

@@ -11,4 +11,13 @@ CREATE ROLE nosuper WITH LOGIN;
 \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper
 -- => ERROR
 \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check
+
+GRANT ALL ON ALL TABLES IN SCHEMA repack TO nosuper;
+GRANT USAGE ON SCHEMA repack TO nosuper;
+
+-- => ERROR
+\! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check
+
+REVOKE ALL ON ALL TABLES IN SCHEMA repack FROM nosuper;
+REVOKE USAGE ON SCHEMA repack FROM nosuper;
 DROP ROLE IF EXISTS nosuper;