Hi,

Am Mittwoch, dem 08.09.2021 um 07:38 +0000 schrieb
shinya11.k...@nttdata.com:
> > Thanks for letting me know, I've attached a rebased v4 of this
> > patch, no other changes.

Please find attached another rebase, sorry it took so long.

I tried it, but when I used set command, tab completion did not work >
properly and an error occurred.

It's been a while, but I believe the patch only changes the ALTER ROLE
command, not the SET/SHOW commands. I thought that was more generally
useful, can you explain the SET use-case?

> ---
> postgres=> \conninfo
> You are connected to database "postgres" as user "aaa" via socket in
> "/tmp" at port "5432".
> postgres=> \du
>                                            List of roles
>  Role name |                        
> Attributes                         |         Member of         
> -----------+---------------------------------------------------------
> ---+---------------------------
>  aaa      
> |                                                            |
> {pg_change_role_settings}
>  penguin   | Superuser, Create role, Create DB, Replication, Bypass
> RLS | {}
> postgres=> show log
> log_autovacuum_min_duration        log_executor_stats                
> log_min_error_statement            log_replication_commands          
> log_timezone
> log_checkpoints                    log_file_mode                     
> log_min_messages                   log_rotation_age                  
> log_transaction_sample_rate
> log_connections                    log_hostname                      
> log_parameter_max_length           log_rotation_size                 
> log_truncate_on_rotation
> log_destination                    log_line_prefix                   
> log_parameter_max_length_on_error  log_statement                     
> logging_collector
> log_disconnections                 log_lock_waits                    
> log_parser_stats                   log_statement_sample_rate         
> logical_decoding_work_mem
> log_duration                       log_min_duration_sample           
> log_planner_stats                  log_statement_stats               
> log_error_verbosity                log_min_duration_statement        
> log_recovery_conflict_waits        log_temp_files                    
> postgres=> show log_duration ;
>  log_duration 
> --------------
>  off
> (1 row)
> 
> postgres=> set log
> log_parameter_max_length_on_error  logical_decoding_work_mem
> postgres=> set log_duration to on;
> 2021-09-08 16:23:39.216 JST [533860] ERROR:  permission denied to set
> parameter "log_duration"
> 2021-09-08 16:23:39.216 JST [533860] STATEMENT:  set log_duration to
> on;
> ERROR:  permission denied to set parameter "log_duration"

So this would work:

postgres=> SHOW ROLE; 
     role     
--------------
 rolesetadmin
(1 row)

postgres=> \du         
                                             List of roles
  Role name   |                         Attributes                         |    
     Member of         
--------------+------------------------------------------------------------+---------------------------
 postgres     | Superuser, Create role, Create DB, Replication, Bypass RLS | {}
 rolesetadmin | Cannot login                                               | 
{pg_change_role_settings}
 test         | Cannot login                                               | {}

postgres=> ALTER ROLE test SET log_statement='all';
ALTER ROLE
postgres=> \drds
          List of settings
 Role | Database |     Settings      
------+----------+-------------------
 test |          | log_statement=all
(1 row)

I am not sure if there is anything to be done about tab completion, can
you clarify here?


Michael

-- 
Michael Banck
Teamleiter PostgreSQL-Team
Projektleiter
Tel.: +49 2166 9901-171
E-Mail: michael.ba...@credativ.de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Geoff Richardson, Peter Lilley

Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz
From 4144cd5ccaa074b04dc4cbec3689f91e19f7f138 Mon Sep 17 00:00:00 2001
From: Michael Banck <michael.ba...@credativ.de>
Date: Sat, 29 Jan 2022 11:44:57 +0100
Subject: [PATCH v5] Add new PGC_ADMINSET guc context and
 pg_change_role_settings predefined role.

This commit introduces `administrator' as a middle ground between `superuser'
and `user' for guc contexts.

Those settings initially include all previously `superuser'-context settings
from the 'Reporting and Logging' ('What/When to Log' but not 'Where to Log')
and both 'Statistics' categories. Further settings could be added in follow-up
commits.

Also, a new predefined role pg_change_role_settings is introduced.  This allows
administrators (that are not superusers) that have been granted this role to
change the above PGC_ADMINSET settings on a per-role basis like "ALTER ROLE
[...] SET log_statement".

The rationale is to allow certain settings that affect logging to be changed
in setups where the DBA has access to the database log, but is not a full
superuser.
---
 doc/src/sgml/ref/alter_role.sgml  | 11 +++---
 doc/src/sgml/user-manag.sgml      |  7 ++++
 src/backend/commands/user.c       |  7 +++-
 src/backend/utils/misc/guc.c      | 61 +++++++++++++++++++------------
 src/include/catalog/pg_authid.dat |  5 +++
 src/include/utils/guc.h           |  1 +
 6 files changed, 62 insertions(+), 30 deletions(-)

diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
index 5aa5648ae7..d332c29c71 100644
--- a/doc/src/sgml/ref/alter_role.sgml
+++ b/doc/src/sgml/ref/alter_role.sgml
@@ -115,11 +115,12 @@ ALTER ROLE { <replaceable class="parameter">role_specification</replaceable> | A
 
   <para>
    Superusers can change anyone's session defaults. Roles having
-   <literal>CREATEROLE</literal> privilege can change defaults for non-superuser
-   roles. Ordinary roles can only set defaults for themselves.
-   Certain configuration variables cannot be set this way, or can only be
-   set if a superuser issues the command.  Only superusers can change a setting
-   for all roles in all databases.
+   <literal>CREATEROLE</literal> privilege or which are a member of the
+   <literal>pg_change_role_settings</literal> predefined role can change
+   defaults for non-superuser roles. Ordinary roles can only set defaults for
+   themselves.  Certain configuration variables cannot be set this way, or can
+   only be set if a superuser issues the command.  Only superusers can change a
+   setting for all roles in all databases.
   </para>
  </refsect1>
 
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 9067be1d9c..4d330b203b 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -542,6 +542,13 @@ DROP ROLE doomed_role;
        <entry>Read all configuration variables, even those normally visible only to
        superusers.</entry>
       </row>
+      <row>
+       <entry>pg_change_role_settings</entry>
+       <entry>Allow changing role-based settings for all non-superuser roles
+       with <literal>ALTER ROLE <replaceable>rolename</replaceable> SET
+       <replaceable>varname</replaceable> TO
+       <replaceable>value</replaceable></literal>.</entry>
+      </row>
       <row>
        <entry>pg_read_all_stats</entry>
        <entry>Read all pg_stat_* views and use various statistics related extensions,
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index f9d3c1246b..cb341102bd 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -849,7 +849,8 @@ AlterRoleSet(AlterRoleSetStmt *stmt)
 
 		/*
 		 * To mess with a superuser you gotta be superuser; else you need
-		 * createrole, or just want to change your own settings
+		 * createrole, the pg_change_role_settings default role, or just want
+		 * to change your own settings.
 		 */
 		if (roleform->rolsuper)
 		{
@@ -860,7 +861,9 @@ AlterRoleSet(AlterRoleSetStmt *stmt)
 		}
 		else
 		{
-			if (!have_createrole_privilege() && roleid != GetUserId())
+			if (!have_createrole_privilege() &&
+				roleid != GetUserId() &&
+				!is_member_of_role(GetUserId(), ROLE_PG_CHANGE_ROLE_SETTINGS))
 				ereport(ERROR,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 						 errmsg("permission denied")));
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 4c94f09c64..3de3963a4e 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -697,6 +697,7 @@ const char *const GucContext_Names[] =
 	 /* PGC_SU_BACKEND */ "superuser-backend",
 	 /* PGC_BACKEND */ "backend",
 	 /* PGC_SUSET */ "superuser",
+	 /* PGC_ADMINSET */ "administrator",
 	 /* PGC_USERSET */ "user"
 };
 
@@ -1373,7 +1374,7 @@ static struct config_bool ConfigureNamesBool[] =
 		NULL, NULL, NULL
 	},
 	{
-		{"log_replication_commands", PGC_SUSET, LOGGING_WHAT,
+		{"log_replication_commands", PGC_ADMINSET, LOGGING_WHAT,
 			gettext_noop("Logs each replication command."),
 			NULL
 		},
@@ -1426,7 +1427,7 @@ static struct config_bool ConfigureNamesBool[] =
 	},
 
 	{
-		{"log_duration", PGC_SUSET, LOGGING_WHAT,
+		{"log_duration", PGC_ADMINSET, LOGGING_WHAT,
 			gettext_noop("Logs the duration of each completed SQL statement."),
 			NULL
 		},
@@ -1471,7 +1472,7 @@ static struct config_bool ConfigureNamesBool[] =
 		NULL, NULL, NULL
 	},
 	{
-		{"log_parser_stats", PGC_SUSET, STATS_MONITORING,
+		{"log_parser_stats", PGC_ADMINSET, STATS_MONITORING,
 			gettext_noop("Writes parser performance statistics to the server log."),
 			NULL
 		},
@@ -1480,7 +1481,7 @@ static struct config_bool ConfigureNamesBool[] =
 		check_stage_log_stats, NULL, NULL
 	},
 	{
-		{"log_planner_stats", PGC_SUSET, STATS_MONITORING,
+		{"log_planner_stats", PGC_ADMINSET, STATS_MONITORING,
 			gettext_noop("Writes planner performance statistics to the server log."),
 			NULL
 		},
@@ -1489,7 +1490,7 @@ static struct config_bool ConfigureNamesBool[] =
 		check_stage_log_stats, NULL, NULL
 	},
 	{
-		{"log_executor_stats", PGC_SUSET, STATS_MONITORING,
+		{"log_executor_stats", PGC_ADMINSET, STATS_MONITORING,
 			gettext_noop("Writes executor performance statistics to the server log."),
 			NULL
 		},
@@ -1498,7 +1499,7 @@ static struct config_bool ConfigureNamesBool[] =
 		check_stage_log_stats, NULL, NULL
 	},
 	{
-		{"log_statement_stats", PGC_SUSET, STATS_MONITORING,
+		{"log_statement_stats", PGC_ADMINSET, STATS_MONITORING,
 			gettext_noop("Writes cumulative performance statistics to the server log."),
 			NULL
 		},
@@ -1520,7 +1521,7 @@ static struct config_bool ConfigureNamesBool[] =
 #endif
 
 	{
-		{"track_activities", PGC_SUSET, STATS_COLLECTOR,
+		{"track_activities", PGC_ADMINSET, STATS_COLLECTOR,
 			gettext_noop("Collects information about executing commands."),
 			gettext_noop("Enables the collection of information on the currently "
 						 "executing command of each session, along with "
@@ -1531,7 +1532,7 @@ static struct config_bool ConfigureNamesBool[] =
 		NULL, NULL, NULL
 	},
 	{
-		{"track_counts", PGC_SUSET, STATS_COLLECTOR,
+		{"track_counts", PGC_ADMINSET, STATS_COLLECTOR,
 			gettext_noop("Collects statistics on database activity."),
 			NULL
 		},
@@ -1540,7 +1541,7 @@ static struct config_bool ConfigureNamesBool[] =
 		NULL, NULL, NULL
 	},
 	{
-		{"track_io_timing", PGC_SUSET, STATS_COLLECTOR,
+		{"track_io_timing", PGC_ADMINSET, STATS_COLLECTOR,
 			gettext_noop("Collects timing statistics for database I/O activity."),
 			NULL
 		},
@@ -1637,7 +1638,7 @@ static struct config_bool ConfigureNamesBool[] =
 #endif
 
 	{
-		{"log_lock_waits", PGC_SUSET, LOGGING_WHAT,
+		{"log_lock_waits", PGC_ADMINSET, LOGGING_WHAT,
 			gettext_noop("Logs long lock waits."),
 			NULL
 		},
@@ -2983,7 +2984,7 @@ static struct config_int ConfigureNamesInt[] =
 	},
 
 	{
-		{"log_min_duration_sample", PGC_SUSET, LOGGING_WHEN,
+		{"log_min_duration_sample", PGC_ADMINSET, LOGGING_WHEN,
 			gettext_noop("Sets the minimum execution time above which "
 						 "a sample of statements will be logged."
 						 " Sampling is determined by log_statement_sample_rate."),
@@ -2996,7 +2997,7 @@ static struct config_int ConfigureNamesInt[] =
 	},
 
 	{
-		{"log_min_duration_statement", PGC_SUSET, LOGGING_WHEN,
+		{"log_min_duration_statement", PGC_ADMINSET, LOGGING_WHEN,
 			gettext_noop("Sets the minimum execution time above which "
 						 "all statements will be logged."),
 			gettext_noop("Zero prints all queries. -1 turns this feature off."),
@@ -3020,7 +3021,7 @@ static struct config_int ConfigureNamesInt[] =
 	},
 
 	{
-		{"log_parameter_max_length", PGC_SUSET, LOGGING_WHAT,
+		{"log_parameter_max_length", PGC_ADMINSET, LOGGING_WHAT,
 			gettext_noop("Sets the maximum length in bytes of data logged for bind "
 						 "parameter values when logging statements."),
 			gettext_noop("-1 to print values in full."),
@@ -3500,7 +3501,7 @@ static struct config_int ConfigureNamesInt[] =
 	},
 
 	{
-		{"log_temp_files", PGC_SUSET, LOGGING_WHAT,
+		{"log_temp_files", PGC_ADMINSET, LOGGING_WHAT,
 			gettext_noop("Log the use of temporary files larger than this number of kilobytes."),
 			gettext_noop("Zero logs all files. The default is -1 (turning this feature off)."),
 			GUC_UNIT_KB
@@ -3849,7 +3850,7 @@ static struct config_real ConfigureNamesReal[] =
 	},
 
 	{
-		{"log_statement_sample_rate", PGC_SUSET, LOGGING_WHEN,
+		{"log_statement_sample_rate", PGC_ADMINSET, LOGGING_WHEN,
 			gettext_noop("Fraction of statements exceeding log_min_duration_sample to be logged."),
 			gettext_noop("Use a value between 0.0 (never log) and 1.0 (always log).")
 		},
@@ -3859,9 +3860,10 @@ static struct config_real ConfigureNamesReal[] =
 	},
 
 	{
-		{"log_transaction_sample_rate", PGC_SUSET, LOGGING_WHEN,
-			gettext_noop("Sets the fraction of transactions from which to log all statements."),
-			gettext_noop("Use a value between 0.0 (never log) and 1.0 (log all "
+		{"log_transaction_sample_rate", PGC_ADMINSET, LOGGING_WHEN,
+			gettext_noop("Sets the fraction of transactions to log for new transactions."),
+			gettext_noop("Logs all statements from a fraction of transactions. "
+						 "Use a value between 0.0 (never log) and 1.0 (log all "
 						 "statements for all transactions).")
 		},
 		&log_xact_sample_rate,
@@ -4754,7 +4756,7 @@ static struct config_enum ConfigureNamesEnum[] =
 	},
 
 	{
-		{"log_error_verbosity", PGC_SUSET, LOGGING_WHAT,
+		{"log_error_verbosity", PGC_ADMINSET, LOGGING_WHAT,
 			gettext_noop("Sets the verbosity of logged messages."),
 			NULL
 		},
@@ -4764,7 +4766,7 @@ static struct config_enum ConfigureNamesEnum[] =
 	},
 
 	{
-		{"log_min_messages", PGC_SUSET, LOGGING_WHEN,
+		{"log_min_messages", PGC_ADMINSET, LOGGING_WHEN,
 			gettext_noop("Sets the message levels that are logged."),
 			gettext_noop("Each level includes all the levels that follow it. The later"
 						 " the level, the fewer messages are sent.")
@@ -4775,7 +4777,7 @@ static struct config_enum ConfigureNamesEnum[] =
 	},
 
 	{
-		{"log_min_error_statement", PGC_SUSET, LOGGING_WHEN,
+		{"log_min_error_statement", PGC_ADMINSET, LOGGING_WHEN,
 			gettext_noop("Causes all statements generating error at or above this level to be logged."),
 			gettext_noop("Each level includes all the levels that follow it. The later"
 						 " the level, the fewer messages are sent.")
@@ -4786,7 +4788,7 @@ static struct config_enum ConfigureNamesEnum[] =
 	},
 
 	{
-		{"log_statement", PGC_SUSET, LOGGING_WHAT,
+		{"log_statement", PGC_ADMINSET, LOGGING_WHAT,
 			gettext_noop("Sets the type of statements logged."),
 			NULL
 		},
@@ -4868,7 +4870,7 @@ static struct config_enum ConfigureNamesEnum[] =
 	},
 
 	{
-		{"track_functions", PGC_SUSET, STATS_COLLECTOR,
+		{"track_functions", PGC_ADMINSET, STATS_COLLECTOR,
 			gettext_noop("Collects function-level statistics on database activity."),
 			NULL
 		},
@@ -7509,6 +7511,19 @@ set_config_option(const char *name, const char *value,
 				return 0;
 			}
 			break;
+		case PGC_ADMINSET:
+			if ((context == PGC_USERSET &&
+				!((source == PGC_S_DATABASE_USER || source == PGC_S_TEST) &&
+				  is_member_of_role(GetUserId(), ROLE_PG_CHANGE_ROLE_SETTINGS))) ||
+				context == PGC_BACKEND)
+			{
+				ereport(elevel,
+						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+						 errmsg("permission denied to set parameter \"%s\"",
+								name)));
+				return 0;
+			}
+			break;
 		case PGC_USERSET:
 			/* always okay */
 			break;
diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat
index 6c28119fa1..2e0d1a1a9f 100644
--- a/src/include/catalog/pg_authid.dat
+++ b/src/include/catalog/pg_authid.dat
@@ -84,5 +84,10 @@
   rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
   rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
   rolpassword => '_null_', rolvaliduntil => '_null_' },
+{ oid => '8801', oid_symbol => 'ROLE_PG_CHANGE_ROLE_SETTINGS',
+  rolname => 'pg_change_role_settings', rolsuper => 'f', rolinherit => 't',
+  rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+  rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
+  rolpassword => '_null_', rolvaliduntil => '_null_' },
 
 ]
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 6bb81707b0..d1091e7fe2 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -73,6 +73,7 @@ typedef enum
 	PGC_SU_BACKEND,
 	PGC_BACKEND,
 	PGC_SUSET,
+	PGC_ADMINSET,
 	PGC_USERSET
 } GucContext;
 
-- 
2.30.2

Reply via email to