On 2022-04-13 17:35, Kyotaro Horiguchi wrote:
At Wed, 13 Apr 2022 16:10:01 +0900, Michael Paquier
<mich...@paquier.xyz> wrote in
On Wed, Apr 13, 2022 at 03:46:25PM +0900, Kyotaro Horiguchi wrote:
> It is sensible to rig createuser command with full capability of
> CREATE ROLE is reasonable.
>
> Only --replication is added by commit 9b8aff8c19 (2010) since
> 8ae0d476a9 (2005). BYPASSRLS and NOBYPASSRLS were introduced by
> 491c029dbc (2014) but it seems to have forgotten to add the
> corresponding createuser options.
>
> By a quick search, found a few other CREATE ROLE optinos that are not
> supported by createuser.
My question is: is BYPASSRLS common enough to justify having a switch
to createuser? As the development cycle of 15 has just finished and
that we are in feature freeze, you may want to hold on new patches for
a bit. The next commit fest is planned for July.
I don't think there's a definitive criteria (other than feasibility)
for whether each CREATE ROLE option should have the correspondent
option in the createuser command. I don't see a clear reason why
createuser command should not have the option.
Thank you for the review!
I created a new patch containing 'VALID UNTIL', 'ADMIN', and 'ROLE'.
To add the ROLE clause, the originally existing --role option
(corresponding to the IN ROLE clause) is changed to the --in-role
option. Would this not be good from a backward compatibility standpoint?
As far as schedules are concerned, I don't think this has anything to
do with 15.
I have registered this patch for the July commit fest.
--
Regards,
--
Shinya Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml
index 17579e50af..fc477605f6 100644
--- a/doc/src/sgml/ref/createuser.sgml
+++ b/doc/src/sgml/ref/createuser.sgml
@@ -76,6 +76,20 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-a <replaceable class="parameter">role</replaceable></option></term>
+ <term><option>--admin=<replaceable class="parameter">role</replaceable></option></term>
+ <listitem>
+ <para>
+ The new role will be added immediately as a member with admin option
+ of this role.
+ Multiple roles to which new role will be added as a member with admin
+ option can be specified by writing multiple
+ <option>-a</option> switches.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-c <replaceable class="parameter">number</replaceable></option></term>
<term><option>--connection-limit=<replaceable class="parameter">number</replaceable></option></term>
@@ -132,7 +146,7 @@ PostgreSQL documentation
<varlistentry>
<term><option>-g <replaceable class="parameter">role</replaceable></option></term>
- <term><option>--role=<replaceable class="parameter">role</replaceable></option></term>
+ <term><option>--in-role=<replaceable class="parameter">role</replaceable></option></term>
<listitem>
<para>
Indicates role to which this role will be added immediately as a new
@@ -143,6 +157,19 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-G <replaceable class="parameter">role</replaceable></option></term>
+ <term><option>--role=<replaceable class="parameter">role</replaceable></option></term>
+ <listitem>
+ <para>
+ The new role will be added immediately as a member of this role.
+ Multiple roles to which new role will be added as a member
+ can be specified by writing multiple
+ <option>-G</option> switches.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-i</option></term>
<term><option>--inherit</option></term>
@@ -258,6 +285,17 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-v <replaceable class="parameter">timestamp</replaceable></option></term>
+ <term><option>--valid-until=<replaceable class="parameter">timestamp</replaceable></option></term>
+ <listitem>
+ <para>
+ Set a timestamp after which the role's password is no longer valid.
+ The default is to set no expiration.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-V</option></term>
<term><option>--version</option></term>
@@ -290,6 +328,28 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--bypassrls</option></term>
+ <listitem>
+ <para>
+ The new user will have the <literal>BYPASSRLS</literal> privilege,
+ which is described more fully in the documentation for <xref
+ linkend="sql-createrole"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><option>--no-bypassrls</option></term>
+ <listitem>
+ <para>
+ The new user will not have the <literal>BYPASSRLS</literal>
+ privilege, which is described more fully in the documentation for <xref
+ linkend="sql-createrole"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-?</option></term>
<term><option>--help</option></term>
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 7309ebddea..3e8d84de36 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -587,7 +587,7 @@ ok(-f "$tempdir/backuponserver/base.tar", 'backup tar was created');
rmtree("$tempdir/backuponserver");
$node->command_ok(
- [qw(createuser --replication --role=pg_write_server_files backupuser)],
+ [qw(createuser --replication --in-role=pg_write_server_files backupuser)],
'create backup user');
$node->command_ok(
[ @pg_basebackup_defs, '-U', 'backupuser', '--target', "server:$tempdir/backuponserver", '-X', 'none' ],
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index bfba0d09d1..0a31593055 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -31,7 +31,7 @@ main(int argc, char *argv[])
{"host", required_argument, NULL, 'h'},
{"port", required_argument, NULL, 'p'},
{"username", required_argument, NULL, 'U'},
- {"role", required_argument, NULL, 'g'},
+ {"in-role", required_argument, NULL, 'g'},
{"no-password", no_argument, NULL, 'w'},
{"password", no_argument, NULL, 'W'},
{"echo", no_argument, NULL, 'e'},
@@ -51,7 +51,12 @@ main(int argc, char *argv[])
{"connection-limit", required_argument, NULL, 'c'},
{"pwprompt", no_argument, NULL, 'P'},
{"encrypted", no_argument, NULL, 'E'},
- {NULL, 0, NULL, 0}
+ {"bypassrls", no_argument, NULL, 4},
+ {"no-bypassrls", no_argument, NULL, 5},
+ {"valid-until", required_argument, NULL, 'v'},
+ {"role", required_argument, NULL, 'G'},
+ {"admin", required_argument, NULL, 'a'},
+ {NULL, 0, NULL, 0},
};
const char *progname;
@@ -69,6 +74,9 @@ main(int argc, char *argv[])
int conn_limit = -2; /* less than minimum valid value */
bool pwprompt = false;
char *newpassword = NULL;
+ SimpleStringList in_roles = {NULL, NULL};
+ SimpleStringList admins = {NULL, NULL};
+ char *timestamp = NULL;
/* Tri-valued variables. */
enum trivalue createdb = TRI_DEFAULT,
@@ -76,7 +84,8 @@ main(int argc, char *argv[])
createrole = TRI_DEFAULT,
inherit = TRI_DEFAULT,
login = TRI_DEFAULT,
- replication = TRI_DEFAULT;
+ replication = TRI_DEFAULT,
+ bypassrls = TRI_DEFAULT;
PQExpBufferData sql;
@@ -89,7 +98,7 @@ main(int argc, char *argv[])
handle_help_version_opts(argc, argv, "createuser", help);
- while ((c = getopt_long(argc, argv, "h:p:U:g:wWedDsSrRiIlLc:PE",
+ while ((c = getopt_long(argc, argv, "h:p:U:g:wWedDsSrRiIlLc:PEv:G:a:",
long_options, &optindex)) != -1)
{
switch (c)
@@ -104,7 +113,7 @@ main(int argc, char *argv[])
username = pg_strdup(optarg);
break;
case 'g':
- simple_string_list_append(&roles, optarg);
+ simple_string_list_append(&in_roles, optarg);
break;
case 'w':
prompt_password = TRI_NO;
@@ -165,6 +174,21 @@ main(int argc, char *argv[])
case 3:
interactive = true;
break;
+ case 4:
+ bypassrls = TRI_YES;
+ break;
+ case 5:
+ bypassrls = TRI_NO;
+ break;
+ case 'v':
+ timestamp = pg_strdup(optarg);
+ break;
+ case 'G':
+ simple_string_list_append(&roles, optarg);
+ break;
+ case 'a':
+ simple_string_list_append(&admins, optarg);
+ break;
default:
/* getopt_long already emitted a complaint */
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -304,14 +328,34 @@ main(int argc, char *argv[])
appendPQExpBufferStr(&sql, " REPLICATION");
if (replication == TRI_NO)
appendPQExpBufferStr(&sql, " NOREPLICATION");
+ if (bypassrls == TRI_YES)
+ appendPQExpBufferStr(&sql, " BYPASSRLS");
+ if (bypassrls == TRI_NO)
+ appendPQExpBufferStr(&sql, " NOBYPASSRLS");
if (conn_limit >= -1)
appendPQExpBuffer(&sql, " CONNECTION LIMIT %d", conn_limit);
- if (roles.head != NULL)
+ if (timestamp != NULL)
+ appendPQExpBuffer(&sql, " VALID UNTIL '%s'", timestamp);
+ if (in_roles.head != NULL)
{
SimpleStringListCell *cell;
appendPQExpBufferStr(&sql, " IN ROLE ");
+ for (cell = in_roles.head; cell; cell = cell->next)
+ {
+ if (cell->next)
+ appendPQExpBuffer(&sql, "%s,", fmtId(cell->val));
+ else
+ appendPQExpBufferStr(&sql, fmtId(cell->val));
+ }
+ }
+ if (roles.head != NULL)
+ {
+ SimpleStringListCell *cell;
+
+ appendPQExpBufferStr(&sql, " ROLE ");
+
for (cell = roles.head; cell; cell = cell->next)
{
if (cell->next)
@@ -320,6 +364,21 @@ main(int argc, char *argv[])
appendPQExpBufferStr(&sql, fmtId(cell->val));
}
}
+ if (admins.head != NULL)
+ {
+ SimpleStringListCell *cell;
+
+ appendPQExpBufferStr(&sql, " ADMIN ");
+
+ for (cell = admins.head; cell; cell = cell->next)
+ {
+ if (cell->next)
+ appendPQExpBuffer(&sql, "%s,", fmtId(cell->val));
+ else
+ appendPQExpBufferStr(&sql, fmtId(cell->val));
+ }
+ }
+
appendPQExpBufferChar(&sql, ';');
if (echo)
@@ -346,11 +405,14 @@ help(const char *progname)
printf(_("Usage:\n"));
printf(_(" %s [OPTION]... [ROLENAME]\n"), progname);
printf(_("\nOptions:\n"));
+ printf(_(" -a, --admin=ROLE this role will be a member of new role\n"
+ " with admin option\n"));
printf(_(" -c, --connection-limit=N connection limit for role (default: no limit)\n"));
printf(_(" -d, --createdb role can create new databases\n"));
printf(_(" -D, --no-createdb role cannot create databases (default)\n"));
printf(_(" -e, --echo show the commands being sent to the server\n"));
- printf(_(" -g, --role=ROLE new role will be a member of this role\n"));
+ printf(_(" -g, --in-role=ROLE new role will be a member of this role\n"));
+ printf(_(" -G, --role=ROLE this role will be a member of new role\n"));
printf(_(" -i, --inherit role inherits privileges of roles it is a\n"
" member of (default)\n"));
printf(_(" -I, --no-inherit role does not inherit privileges\n"));
@@ -361,11 +423,14 @@ help(const char *progname)
printf(_(" -R, --no-createrole role cannot create roles (default)\n"));
printf(_(" -s, --superuser role will be superuser\n"));
printf(_(" -S, --no-superuser role will not be superuser (default)\n"));
+ printf(_(" -v, --valid-until password expiration timestamp for role\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" --interactive prompt for missing role name and attributes rather\n"
" than using defaults\n"));
printf(_(" --replication role can initiate replication\n"));
printf(_(" --no-replication role cannot initiate replication\n"));
+ printf(_(" --bypassrls role can bypass row-level security (RLS) policy\n"));
+ printf(_(" --no-bypassrls role cannot bypass row-level security (RLS) policy\n"));
printf(_(" -?, --help show this help, then exit\n"));
printf(_("\nConnection options:\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
