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