Subject: [PATCH v6 2/2] Improve more insufficient-privileges error messages.

From: Nathan Bossart <[email-masked]>
Date: Fri, 27 Jan 2023 14:52:57 -0800

---
 contrib/file_fdw/expected/file_fdw.out           |  3 ++-
 contrib/file_fdw/file_fdw.c                      |  8 ++++++--
 contrib/test_decoding/expected/permissions.out   | 12 ++++++++----
 src/backend/backup/basebackup_server.c           |  4 +++-
 src/backend/catalog/objectaddress.c              | 16 +++++++++++-----
 src/backend/commands/copy.c                      | 12 +++++++++---
 src/backend/replication/slot.c                   |  6 ++++--
 src/backend/storage/ipc/procarray.c              |  4 +++-
 src/backend/storage/ipc/signalfuncs.c            | 16 ++++++++++++----
 src/backend/tcop/utility.c                       |  5 ++++-
 src/backend/utils/init/miscinit.c                |  4 ++++
 src/backend/utils/init/postinit.c                | 12 ++++++++----
 src/backend/utils/misc/guc.c                     | 15 +++++++++------
 .../dummy_seclabel/expected/dummy_seclabel.out   |  3 ++-
 .../modules/unsafe_tests/expected/rolenames.out  |  3 ++-
 src/test/regress/expected/create_role.out        |  3 ++-
 16 files changed, 89 insertions(+), 37 deletions(-)

diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out
index 36d76ba26c..ea7afd6fe8 100644
--- a/contrib/file_fdw/expected/file_fdw.out
+++ b/contrib/file_fdw/expected/file_fdw.out
@@ -474,7 +474,8 @@ ALTER FOREIGN TABLE agg_text OWNER TO regress_file_fdw_user;
 ALTER FOREIGN TABLE agg_text OPTIONS (SET format 'text');
 SET ROLE regress_file_fdw_user;
 ALTER FOREIGN TABLE agg_text OPTIONS (SET format 'text');
-ERROR:  only superuser or a role with privileges of the pg_read_server_files role may specify the filename option of a file_fdw foreign table
+ERROR:  permission denied to set the "filename" option of a file_fdw foreign table
+DETAIL:  You must inherit privileges of the "pg_read_server_files" role to set this option.
 SET ROLE regress_file_fdw_superuser;
 -- cleanup
 RESET ROLE;
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 8ccc167548..8553d93b3f 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -278,13 +278,17 @@ file_fdw_validator(PG_FUNCTION_ARGS)
 				!has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
 				ereport(ERROR,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-						 errmsg("only superuser or a role with privileges of the pg_read_server_files role may specify the filename option of a file_fdw foreign table")));
+						 errmsg("permission denied to set the \"filename\" option of a file_fdw foreign table"),
+						 errdetail("You must inherit privileges of the \"%s\" role to set this option.",
+								   "pg_read_server_files")));
 
 			if (strcmp(def->defname, "program") == 0 &&
 				!has_privs_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM))
 				ereport(ERROR,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-						 errmsg("only superuser or a role with privileges of the pg_execute_server_program role may specify the program option of a file_fdw foreign table")));
+						 errmsg("permission denied to set the \"program\" option of a file_fdw foreign table"),
+						 errdetail("You must inherit privileges of the \"%s\" role to set this option.",
+								   "pg_execute_server_program")));
 
 			filename = defGetString(def);
 		}
diff --git a/contrib/test_decoding/expected/permissions.out b/contrib/test_decoding/expected/permissions.out
index ed97f81dda..b45f5cf0df 100644
--- a/contrib/test_decoding/expected/permissions.out
+++ b/contrib/test_decoding/expected/permissions.out
@@ -54,13 +54,16 @@ RESET ROLE;
 -- plain user *can't* can control replication
 SET ROLE regress_lr_normal;
 SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_decoding');
-ERROR:  must be superuser or replication role to use replication slots
+ERROR:  permission denied to use replication slots
+DETAIL:  You must have the REPLICATION attribute to use replication slots.
 INSERT INTO lr_test VALUES('lr_superuser_init');
 ERROR:  permission denied for table lr_test
 SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
-ERROR:  must be superuser or replication role to use replication slots
+ERROR:  permission denied to use replication slots
+DETAIL:  You must have the REPLICATION attribute to use replication slots.
 SELECT pg_drop_replication_slot('regression_slot');
-ERROR:  must be superuser or replication role to use replication slots
+ERROR:  permission denied to use replication slots
+DETAIL:  You must have the REPLICATION attribute to use replication slots.
 RESET ROLE;
 -- replication users can drop superuser created slots
 SET ROLE regress_lr_superuser;
@@ -90,7 +93,8 @@ SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot', 'test_d
 RESET ROLE;
 SET ROLE regress_lr_normal;
 SELECT pg_drop_replication_slot('regression_slot');
-ERROR:  must be superuser or replication role to use replication slots
+ERROR:  permission denied to use replication slots
+DETAIL:  You must have the REPLICATION attribute to use replication slots.
 RESET ROLE;
 -- all users can see existing slots
 SET ROLE regress_lr_superuser;
diff --git a/src/backend/backup/basebackup_server.c b/src/backend/backup/basebackup_server.c
index 0258d7a03b..aac64408d1 100644
--- a/src/backend/backup/basebackup_server.c
+++ b/src/backend/backup/basebackup_server.c
@@ -72,7 +72,9 @@ bbsink_server_new(bbsink *next, char *pathname)
 	if (!has_privs_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be superuser or a role with privileges of the pg_write_server_files role to create backup stored on server")));
+				 errmsg("permission denied to create backup stored on server"),
+				 errdetail("You must inherit privileges of the \"%s\" role.",
+						   "pg_write_server_files")));
 	CommitTransactionCommand();
 
 	/*
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 2f688166e1..c1ab4ea161 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -2547,20 +2547,26 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
 				if (!superuser_arg(roleid))
 					ereport(ERROR,
 							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-							 errmsg("must be superuser")));
+							 errmsg("permission denied"),
+							 errdetail("You must have the %s attribute.",
+									   "SUPERUSER")));
 			}
 			else
 			{
 				if (!has_createrole_privilege(roleid))
 					ereport(ERROR,
 							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-							 errmsg("must have CREATEROLE privilege")));
+							 errmsg("permission denied"),
+							 errdetail("You must have the %s attribute.",
+									   "CREATEROLE")));
 				if (!is_admin_of_role(roleid, address.objectId))
 					ereport(ERROR,
 							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-							 errmsg("must have admin option on role \"%s\"",
-									GetUserNameFromId(address.objectId,
-													  true))));
+							 errmsg("permission denied"),
+							 errdetail("You must have the %s option on role \"%s\".",
+									   "ADMIN",
+									   GetUserNameFromId(address.objectId,
+														 true))));
 			}
 			break;
 		case OBJECT_TSPARSER:
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index e34f583ea7..388cfd7cae 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -83,7 +83,9 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
 			if (!has_privs_of_role(GetUserId(), ROLE_PG_EXECUTE_SERVER_PROGRAM))
 				ereport(ERROR,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-						 errmsg("must be superuser or have privileges of the pg_execute_server_program role to COPY to or from an external program"),
+						 errmsg("permission denied to COPY to or from an external program"),
+						 errdetail("You must inherit privileges of the \"%s\" role.",
+								   "pg_execute_server_program"),
 						 errhint("Anyone can COPY to stdout or from stdin. "
 								 "psql's \\copy command also works for anyone.")));
 		}
@@ -92,14 +94,18 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
 			if (is_from && !has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
 				ereport(ERROR,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-						 errmsg("must be superuser or have privileges of the pg_read_server_files role to COPY from a file"),
+						 errmsg("permission denied to COPY from a file"),
+						 errdetail("You must inherit privileges of the \"%s\" role.",
+								   "pg_read_server_files"),
 						 errhint("Anyone can COPY to stdout or from stdin. "
 								 "psql's \\copy command also works for anyone.")));
 
 			if (!is_from && !has_privs_of_role(GetUserId(), ROLE_PG_WRITE_SERVER_FILES))
 				ereport(ERROR,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-						 errmsg("must be superuser or have privileges of the pg_write_server_files role to COPY to a file"),
+						 errmsg("permission denied to COPY to a file"),
+						 errdetail("You must inherit privileges of the \"%s\" role.",
+								   "pg_write_server_files"),
 						 errhint("Anyone can COPY to stdout or from stdin. "
 								 "psql's \\copy command also works for anyone.")));
 		}
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index f286918f69..51adcd8418 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1140,10 +1140,12 @@ CheckSlotRequirements(void)
 void
 CheckSlotPermissions(void)
 {
-	if (!superuser() && !has_rolreplication(GetUserId()))
+	if (!has_rolreplication(GetUserId()))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be superuser or replication role to use replication slots")));
+				 errmsg("permission denied to use replication slots"),
+				 errdetail("You must have the %s attribute to use replication slots.",
+						   "REPLICATION")));
 }
 
 /*
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index a7071b2fce..09a69f2c76 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -3860,7 +3860,9 @@ TerminateOtherDBBackends(Oid databaseId)
 					!has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_BACKEND))
 					ereport(ERROR,
 							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-							 errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend")));
+							 errmsg("permission denied to terminate process"),
+							 errdetail("You must inherit privileges of the role whose process is being terminated or inherit privileges of the \"%s\" role.",
+									   "pg_signal_backend")));
 			}
 		}
 
diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c
index bc93ab5b52..c1be67e7d0 100644
--- a/src/backend/storage/ipc/signalfuncs.c
+++ b/src/backend/storage/ipc/signalfuncs.c
@@ -121,12 +121,16 @@ pg_cancel_backend(PG_FUNCTION_ARGS)
 	if (r == SIGNAL_BACKEND_NOSUPERUSER)
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be a superuser to cancel superuser query")));
+				 errmsg("permission denied to cancel query"),
+				 errdetail("You must have the %s attribute to cancel queries of roles with %s.",
+						   "SUPERUSER", "SUPERUSER")));
 
 	if (r == SIGNAL_BACKEND_NOPERMISSION)
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be a member of the role whose query is being canceled or member of pg_signal_backend")));
+				 errmsg("permission denied to cancel query"),
+				 errdetail("You must inherit privileges of the role whose query is being canceled or inherit privileges of the \"%s\" role.",
+						   "pg_signal_backend")));
 
 	PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
 }
@@ -223,12 +227,16 @@ pg_terminate_backend(PG_FUNCTION_ARGS)
 	if (r == SIGNAL_BACKEND_NOSUPERUSER)
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be a superuser to terminate superuser process")));
+				 errmsg("permission denied to terminate process"),
+				 errdetail("You must have the %s attribute to terminate processes of roles with %s.",
+						   "SUPERUSER", "SUPERUSER")));
 
 	if (r == SIGNAL_BACKEND_NOPERMISSION)
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend")));
+				 errmsg("permission denied to terminate process"),
+				 errdetail("You must inherit privileges of the role whose process is being terminated or inherit privileges of the \"%s\" role.",
+						   "pg_signal_backend")));
 
 	/* Wait only on success and if actually requested */
 	if (r == SIGNAL_BACKEND_SUCCESS && timeout > 0)
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index c7d9d96b45..3fd9cfb842 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -950,7 +950,10 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			if (!has_privs_of_role(GetUserId(), ROLE_PG_CHECKPOINT))
 				ereport(ERROR,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-						 errmsg("must be superuser or have privileges of pg_checkpoint to do CHECKPOINT")));
+						 errmsg("permission denied to execute %s command",
+								"CHECKPOINT"),
+						 errdetail("You must inherit privileges of the \"%s\" role to execute this command.",
+								   "pg_checkpoint")));
 
 			RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_WAIT |
 							  (RecoveryInProgress() ? 0 : CHECKPOINT_FORCE));
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 59532bbd80..b0cfbc742b 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -701,6 +701,10 @@ has_rolreplication(Oid roleid)
 	bool		result = false;
 	HeapTuple	utup;
 
+	/* Superusers bypass all permission checking. */
+	if (superuser_arg(roleid))
+		return true;
+
 	utup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
 	if (HeapTupleIsValid(utup))
 	{
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 2f07ca7a0e..2b103586ab 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -940,12 +940,14 @@ InitPostgres(const char *in_dbname, Oid dboid,
 		if (nfree < SuperuserReservedConnections)
 			ereport(FATAL,
 					(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
-					 errmsg("remaining connection slots are reserved for superusers")));
+					 errmsg("remaining connection slots are reserved for roles with %s",
+							"SUPERUSER")));
 
 		if (!has_privs_of_role(GetUserId(), ROLE_PG_USE_RESERVED_CONNECTIONS))
 			ereport(FATAL,
 					(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
-					 errmsg("remaining connection slots are reserved for roles with privileges of pg_use_reserved_connections")));
+					 errmsg("remaining connection slots are reserved for roles that inherit privileges of the \"%s\" role",
+							"pg_use_reserved_connections")));
 	}
 
 	/* Check replication permissions needed for walsender processes. */
@@ -953,10 +955,12 @@ InitPostgres(const char *in_dbname, Oid dboid,
 	{
 		Assert(!bootstrap);
 
-		if (!superuser() && !has_rolreplication(GetUserId()))
+		if (!has_rolreplication(GetUserId()))
 			ereport(FATAL,
 					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-					 errmsg("must be superuser or replication role to start walsender")));
+					 errmsg("permission denied to start WAL sender"),
+					 errdetail("You must have the %s attribute to start a WAL sender process.",
+							   "REPLICATION")));
 	}
 
 	/*
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 51e07d5582..7805a6015b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4209,8 +4209,9 @@ GetConfigOption(const char *name, bool missing_ok, bool restrict_privileged)
 		!ConfigOptionIsVisible(record))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be superuser or have privileges of pg_read_all_settings to examine \"%s\"",
-						name)));
+				 errmsg("permission denied to examine \"%s\"", name),
+				 errdetail("You must inherit privileges of the \"%s\" role to examine this parameter.",
+						   "pg_read_all_settings")));
 
 	switch (record->vartype)
 	{
@@ -4255,8 +4256,9 @@ GetConfigOptionResetString(const char *name)
 	if (!ConfigOptionIsVisible(record))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be superuser or have privileges of pg_read_all_settings to examine \"%s\"",
-						name)));
+				 errmsg("permission denied to examine \"%s\"", name),
+				 errdetail("You must inherit privileges of the \"%s\" role to examine this parameter.",
+						   "pg_read_all_settings")));
 
 	switch (record->vartype)
 	{
@@ -5261,8 +5263,9 @@ GetConfigOptionByName(const char *name, const char **varname, bool missing_ok)
 	if (!ConfigOptionIsVisible(record))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be superuser or have privileges of pg_read_all_settings to examine \"%s\"",
-						name)));
+				 errmsg("permission denied to examine \"%s\"", name),
+				 errdetail("You must inherit privileges of the \"%s\" role to examine this parameter.",
+						   "pg_read_all_settings")));
 
 	if (varname)
 		*varname = record->name;
diff --git a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
index c57d4fd2df..18f8db0e39 100644
--- a/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
+++ b/src/test/modules/dummy_seclabel/expected/dummy_seclabel.out
@@ -59,7 +59,8 @@ SECURITY LABEL ON ROLE regress_dummy_seclabel_user4 IS 'unclassified';	-- fail (
 ERROR:  role "regress_dummy_seclabel_user4" does not exist
 SET SESSION AUTHORIZATION regress_dummy_seclabel_user2;
 SECURITY LABEL ON ROLE regress_dummy_seclabel_user2 IS 'unclassified';	-- fail (not privileged)
-ERROR:  must have CREATEROLE privilege
+ERROR:  permission denied
+DETAIL:  You must have the CREATEROLE attribute.
 RESET SESSION AUTHORIZATION;
 --
 -- Test for various types of object
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index 88b1ff843b..6e55850705 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -1077,7 +1077,8 @@ SHOW session_preload_libraries;
 SET SESSION AUTHORIZATION regress_role_nopriv;
 -- fails with role not member of pg_read_all_settings
 SHOW session_preload_libraries;
-ERROR:  must be superuser or have privileges of pg_read_all_settings to examine "session_preload_libraries"
+ERROR:  permission denied to examine "session_preload_libraries"
+DETAIL:  You must inherit privileges of the "pg_read_all_settings" role to examine this parameter.
 RESET SESSION AUTHORIZATION;
 ERROR:  current transaction is aborted, commands ignored until end of transaction block
 ROLLBACK;
diff --git a/src/test/regress/expected/create_role.out b/src/test/regress/expected/create_role.out
index 7157d4c589..e72eb91624 100644
--- a/src/test/regress/expected/create_role.out
+++ b/src/test/regress/expected/create_role.out
@@ -106,7 +106,8 @@ ALTER ROLE regress_hasprivs RENAME TO regress_tenant;
 ALTER ROLE regress_tenant NOINHERIT NOLOGIN CONNECTION LIMIT 7;
 -- fail, we should be unable to modify a role we did not create
 COMMENT ON ROLE regress_role_normal IS 'some comment';
-ERROR:  must have admin option on role "regress_role_normal"
+ERROR:  permission denied
+DETAIL:  You must have the ADMIN option on role "regress_role_normal".
 ALTER ROLE regress_role_normal RENAME TO regress_role_abnormal;
 ERROR:  permission denied to rename role
 DETAIL:  You must have the CREATEROLE attribute and the ADMIN option on role "regress_role_normal".
-- 
2.25.1

--CE+1k2dSO48ffgeK--