disable prompting by default in createuser

Started by Peter Eisentrautabout 14 years ago6 messages
#1Peter Eisentraut
peter_e@gmx.net

I propose that we change createuser so that it does not prompt for
anything by default. We can arrange options so that you can get prompts
for whatever is missing, but by default, a call to createuser should
just run CREATE USER with default options. The fact that createuser
issues prompts is always annoying when you create setup scripts, because
you have to be careful to specify all the necessary options, and they
are inconsistent and different between versions (although the last
change about that was a long time ago), and the whole behavior seems
contrary to the behavior of all other utilities. I don't think there'd
be a real loss by not prompting by default; after all, we don't really
want to encourage users to create more superusers, do we? Comments?

#2Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#1)
Re: disable prompting by default in createuser

On Fri, Nov 25, 2011 at 6:28 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

I propose that we change createuser so that it does not prompt for
anything by default.  We can arrange options so that you can get prompts
for whatever is missing, but by default, a call to createuser should
just run CREATE USER with default options.  The fact that createuser
issues prompts is always annoying when you create setup scripts, because
you have to be careful to specify all the necessary options, and they
are inconsistent and different between versions (although the last
change about that was a long time ago), and the whole behavior seems
contrary to the behavior of all other utilities.  I don't think there'd
be a real loss by not prompting by default; after all, we don't really
want to encourage users to create more superusers, do we?  Comments?

+1. I've never found the current behavior to be other than an
annoyance. I suggest --interactive or something like that to enable
prompting.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Peter Eisentraut (#1)
1 attachment(s)
Re: disable prompting by default in createuser

On lör, 2011-11-26 at 01:28 +0200, Peter Eisentraut wrote:

I propose that we change createuser so that it does not prompt for
anything by default. We can arrange options so that you can get prompts
for whatever is missing, but by default, a call to createuser should
just run CREATE USER with default options. The fact that createuser
issues prompts is always annoying when you create setup scripts, because
you have to be careful to specify all the necessary options, and they
are inconsistent and different between versions (although the last
change about that was a long time ago), and the whole behavior seems
contrary to the behavior of all other utilities. I don't think there'd
be a real loss by not prompting by default; after all, we don't really
want to encourage users to create more superusers, do we? Comments?

Patch attached. I'll add it to the next commitfest.

Attachments:

createuser-noprompt.patchtext/x-patch; charset=UTF-8; name=createuser-noprompt.patchDownload
diff --git i/doc/src/sgml/ref/createuser.sgml w/doc/src/sgml/ref/createuser.sgml
index 4cbfd69..90b0fd4 100644
--- i/doc/src/sgml/ref/createuser.sgml
+++ w/doc/src/sgml/ref/createuser.sgml
@@ -102,7 +102,8 @@ PostgreSQL documentation
       <term><option>--no-createdb</></term>
       <listitem>
        <para>
-        The new user will not be allowed to create databases.
+        The new user will not be allowed to create databases.  This is the
+        default.
        </para>
       </listitem>
      </varlistentry>
@@ -153,6 +154,19 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
+      <term><option>--interactive</></term>
+      <listitem>
+       <para>
+        Prompt for the user name if none is specified on the command line, and
+        also prompt for whichever of the
+        options <option>-d</option>/<option>-D</option>, <option>-r</option>/<option>-R</option>, <option>-s</option>/<option>-S</option>
+        is not specified on the command line.  (This was the default behavior
+        up to PostgreSQL 9.1.)
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
       <term><option>-l</></term>
       <term><option>--login</></term>
       <listitem>
@@ -215,7 +229,8 @@ PostgreSQL documentation
       <term><option>--no-createrole</></term>
       <listitem>
        <para>
-        The new user will not be allowed to create new roles.
+        The new user will not be allowed to create new roles.  This is the
+        default.
        </para>
       </listitem>
      </varlistentry>
@@ -235,7 +250,7 @@ PostgreSQL documentation
       <term><option>--no-superuser</></term>
       <listitem>
        <para>
-        The new user will not be a superuser.
+        The new user will not be a superuser.  This is the default.
        </para>
       </listitem>
      </varlistentry>
@@ -287,11 +302,6 @@ PostgreSQL documentation
   </para>
 
   <para>
-   You will be prompted for a name and other missing information if it
-   is not specified on the command line.
-  </para>
-
-  <para>
    <application>createuser</application> also accepts the following
    command-line arguments for connection parameters:
 
@@ -422,6 +432,14 @@ PostgreSQL documentation
     server:
 <screen>
 <prompt>$ </prompt><userinput>createuser joe</userinput>
+</screen>
+   </para>
+
+   <para>
+    To create a user <literal>joe</literal> on the default database
+    server with prompting for some additional attributes:
+<screen>
+<prompt>$ </prompt><userinput>createuser --interactive joe</userinput>
 <computeroutput>Shall the new role be a superuser? (y/n) </computeroutput><userinput>n</userinput>
 <computeroutput>Shall the new role be allowed to create databases? (y/n) </computeroutput><userinput>n</userinput>
 <computeroutput>Shall the new role be allowed to create more new roles? (y/n) </computeroutput><userinput>n</userinput>
@@ -430,7 +448,7 @@ PostgreSQL documentation
 
    <para>
     To create the same user <literal>joe</literal> using the
-    server on host <literal>eden</>, port 5000, avoiding the prompts and
+    server on host <literal>eden</>, port 5000, with attributes explicitly specified,
     taking a look at the underlying command:
 <screen>
 <prompt>$ </prompt><userinput>createuser -h eden -p 5000 -S -D -R -e joe</userinput>
diff --git i/doc/src/sgml/ref/dropuser.sgml w/doc/src/sgml/ref/dropuser.sgml
index 724fe40..bc6feaf 100644
--- i/doc/src/sgml/ref/dropuser.sgml
+++ w/doc/src/sgml/ref/dropuser.sgml
@@ -62,7 +62,9 @@ PostgreSQL documentation
       <listitem>
        <para>
         Specifies the name of the <productname>PostgreSQL</productname> user to be removed.
-        You will be prompted for a name if none is specified on the command line.
+        You will be prompted for a name if none is specified on the command
+        line and the <option>-i</option>/<option>--interactive</option> option
+        is used.
        </para>
       </listitem>
      </varlistentry>
@@ -83,7 +85,8 @@ PostgreSQL documentation
       <term><option>--interactive</></term>
       <listitem>
        <para>
-        Prompt for confirmation before actually removing the user.
+        Prompt for confirmation before actually removing the user, and prompt
+        for the user name if none is specified on the command line.
        </para>
       </listitem>
      </varlistentry>
diff --git i/src/bin/scripts/createuser.c w/src/bin/scripts/createuser.c
index d6e05dd..ce74410 100644
--- i/src/bin/scripts/createuser.c
+++ w/src/bin/scripts/createuser.c
@@ -39,6 +39,7 @@ main(int argc, char *argv[])
 		{"no-login", no_argument, NULL, 'L'},
 		{"replication", no_argument, NULL, 1},
 		{"no-replication", no_argument, NULL, 2},
+		{"interactive", no_argument, NULL, 3},
 		/* adduser is obsolete, undocumented spelling of superuser */
 		{"adduser", no_argument, NULL, 'a'},
 		{"no-adduser", no_argument, NULL, 'A'},
@@ -52,12 +53,13 @@ main(int argc, char *argv[])
 	const char *progname;
 	int			optindex;
 	int			c;
-	char	   *newuser = NULL;
+	const char *newuser = NULL;
 	char	   *host = NULL;
 	char	   *port = NULL;
 	char	   *username = NULL;
 	enum trivalue prompt_password = TRI_DEFAULT;
 	bool		echo = false;
+	bool		interactive = false;
 	char	   *conn_limit = NULL;
 	bool		pwprompt = false;
 	char	   *newpassword = NULL;
@@ -154,6 +156,9 @@ main(int argc, char *argv[])
 			case 2:
 				replication = TRI_NO;
 				break;
+			case 3:
+				interactive = true;
+				break;
 			default:
 				fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
 				exit(1);
@@ -175,7 +180,17 @@ main(int argc, char *argv[])
 	}
 
 	if (newuser == NULL)
-		newuser = simple_prompt("Enter name of role to add: ", 128, true);
+	{
+		if (interactive)
+			newuser = simple_prompt("Enter name of role to add: ", 128, true);
+		else
+		{
+			if (getenv("PGUSER"))
+				newuser = getenv("PGUSER");
+			else
+				newuser = get_user_name(progname);
+		}
+	}
 
 	if (pwprompt)
 	{
@@ -195,7 +210,7 @@ main(int argc, char *argv[])
 
 	if (superuser == 0)
 	{
-		if (yesno_prompt("Shall the new role be a superuser?"))
+		if (interactive && yesno_prompt("Shall the new role be a superuser?"))
 			superuser = TRI_YES;
 		else
 			superuser = TRI_NO;
@@ -210,7 +225,7 @@ main(int argc, char *argv[])
 
 	if (createdb == 0)
 	{
-		if (yesno_prompt("Shall the new role be allowed to create databases?"))
+		if (interactive && yesno_prompt("Shall the new role be allowed to create databases?"))
 			createdb = TRI_YES;
 		else
 			createdb = TRI_NO;
@@ -218,7 +233,7 @@ main(int argc, char *argv[])
 
 	if (createrole == 0)
 	{
-		if (yesno_prompt("Shall the new role be allowed to create more new roles?"))
+		if (interactive && yesno_prompt("Shall the new role be allowed to create more new roles?"))
 			createrole = TRI_YES;
 		else
 			createrole = TRI_NO;
@@ -315,20 +330,22 @@ help(const char *progname)
 	printf(_("\nOptions:\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\n"));
+	printf(_("  -D, --no-createdb         role cannot create databases (default)\n"));
 	printf(_("  -e, --echo                show the commands being sent to the server\n"));
 	printf(_("  -E, --encrypted           encrypt stored password\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"));
+	printf(_("      --interactive         prompt for missing role name and attributes rather\n"
+			 "                            than using defaults\n"));
 	printf(_("  -l, --login               role can login (default)\n"));
 	printf(_("  -L, --no-login            role cannot login\n"));
 	printf(_("  -N, --unencrypted         do not encrypt stored password\n"));
 	printf(_("  -P, --pwprompt            assign a password to new role\n"));
 	printf(_("  -r, --createrole          role can create new roles\n"));
-	printf(_("  -R, --no-createrole       role cannot create roles\n"));
+	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\n"));
+	printf(_("  -S, --no-superuser        role will not be superuser (default)\n"));
 	printf(_("  --replication             role can initiate replication\n"));
 	printf(_("  --no-replication          role cannot initiate replication\n"));
 	printf(_("  --help                    show this help, then exit\n"));
@@ -339,7 +356,5 @@ help(const char *progname)
 	printf(_("  -U, --username=USERNAME   user name to connect as (not the one to create)\n"));
 	printf(_("  -w, --no-password         never prompt for password\n"));
 	printf(_("  -W, --password            force password prompt\n"));
-	printf(_("\nIf one of -d, -D, -r, -R, -s, -S, and ROLENAME is not specified, you will\n"
-			 "be prompted interactively.\n"));
 	printf(_("\nReport bugs to <pgsql-bugs@postgresql.org>.\n"));
 }
diff --git i/src/bin/scripts/dropuser.c w/src/bin/scripts/dropuser.c
index bfd5b1c..15fe834 100644
--- i/src/bin/scripts/dropuser.c
+++ w/src/bin/scripts/dropuser.c
@@ -106,7 +106,16 @@ main(int argc, char *argv[])
 	}
 
 	if (dropuser == NULL)
-		dropuser = simple_prompt("Enter name of role to drop: ", 128, true);
+	{
+		if (interactive)
+			dropuser = simple_prompt("Enter name of role to drop: ", 128, true);
+		else
+		{
+			fprintf(stderr, _("%s: missing required argument role name\n"), progname);
+			fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+			exit(1);
+		}
+	}
 
 	if (interactive)
 	{
@@ -147,7 +156,8 @@ help(const char *progname)
 	printf(_("  %s [OPTION]... [ROLENAME]\n"), progname);
 	printf(_("\nOptions:\n"));
 	printf(_("  -e, --echo                show the commands being sent to the server\n"));
-	printf(_("  -i, --interactive         prompt before deleting anything\n"));
+	printf(_("  -i, --interactive         prompt before deleting anything, and prompt for\n"
+			 "                            role name if not specified\n"));
 	printf(_("  --if-exists               don't report error if user doesn't exist\n"));
 	printf(_("  --help                    show this help, then exit\n"));
 	printf(_("  --version                 output version information, then exit\n"));
#4Josh Kupershmidt
schmiddy@gmail.com
In reply to: Peter Eisentraut (#3)
Re: disable prompting by default in createuser

On Thu, Dec 22, 2011 at 2:26 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On lör, 2011-11-26 at 01:28 +0200, Peter Eisentraut wrote:

I propose that we change createuser so that it does not prompt for
anything by default.  We can arrange options so that you can get prompts
for whatever is missing, but by default, a call to createuser should
just run CREATE USER with default options.  The fact that createuser
issues prompts is always annoying when you create setup scripts, because
you have to be careful to specify all the necessary options, and they
are inconsistent and different between versions (although the last
change about that was a long time ago), and the whole behavior seems
contrary to the behavior of all other utilities.  I don't think there'd
be a real loss by not prompting by default; after all, we don't really
want to encourage users to create more superusers, do we?  Comments?

Patch attached.  I'll add it to the next commitfest.

I looked at this patch for the 2012-01 CF. I like the idea, the
interactive-by-default behavior of createuser has always bugged me as
well.

I see this patch includes a small change to dropuser, to make the
'username' argument mandatory if --interactive is not set, for
symmetry with createuser's new behavior. That's dandy, though IMO we
shouldn't have "-i" be shorthand for "--interactive" with dropuser,
and something different with createuser (i.e. we should just get rid
of the "i" alias for dropuser).

Another little inconsistency I see with the behavior when no username
to create or drop is given:

$ createuser
createuser: creation of new role failed: ERROR: role "josh" already exists
$ dropuser
dropuser: missing required argument role name
Try "dropuser --help" for more information.

i.e. createuser tries taking either $PGUSER or the current username as
a default user to create, while dropuser just bails out. Personally, I
prefer just bailing out if no create/drop user is specified, but
either way I think they should be consistent.

Oh, and I think the leading whitespace of this help message:

printf(_(" --interactive prompt for missing role name
and attributes rather\n"

should be made the same as for other commands with no short-alias, e.g.

printf(_(" --replication role can initiate replication\n"));

Other than those little complaints, everything looks good.

Josh

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Josh Kupershmidt (#4)
Re: disable prompting by default in createuser

On sön, 2012-01-15 at 18:14 -0500, Josh Kupershmidt wrote:

I see this patch includes a small change to dropuser, to make the
'username' argument mandatory if --interactive is not set, for
symmetry with createuser's new behavior. That's dandy, though IMO we
shouldn't have "-i" be shorthand for "--interactive" with dropuser,
and something different with createuser (i.e. we should just get rid
of the "i" alias for dropuser).

Well, all the other tools also support -i for prompting. I'd rather get
rid of -i for --inherit, but I fear that will break things as well. I'm
not sure what to do.

Another little inconsistency I see with the behavior when no username
to create or drop is given:

$ createuser
createuser: creation of new role failed: ERROR: role "josh" already
exists
$ dropuser
dropuser: missing required argument role name
Try "dropuser --help" for more information.

i.e. createuser tries taking either $PGUSER or the current username as
a default user to create, while dropuser just bails out. Personally, I
prefer just bailing out if no create/drop user is specified, but
either way I think they should be consistent.

That is intentional long-standing behavior. createdb/dropdb work the
same way.

#6Josh Kupershmidt
schmiddy@gmail.com
In reply to: Peter Eisentraut (#5)
Re: disable prompting by default in createuser

On Wed, Feb 1, 2012 at 1:13 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On sön, 2012-01-15 at 18:14 -0500, Josh Kupershmidt wrote:

I see this patch includes a small change to dropuser, to make the
'username' argument mandatory if --interactive is not set, for
symmetry with createuser's new behavior. That's dandy, though IMO we
shouldn't have "-i" be shorthand for "--interactive" with dropuser,
and something different with createuser (i.e. we should just get rid
of the "i" alias for dropuser).

Well, all the other tools also support -i for prompting.

Taking a look at the current ./src/bin/scripts executables, I see only
2 out of 9 (`dropdb` and `dropuser`) which have "-i" mean
"--interactive", and `reindexdb` has another meaning for "-i"
entirely. So I'm not sure there's such a clear precedent for having
"-i" mean "--interactive" within our scripts, at least.

I'd rather get
rid of -i for --inherit, but I fear that will break things as well.  I'm
not sure what to do.

I think breaking backwards compatibility probably won't fly (and
should probably be handled by another patch, anyway). I guess it's OK
to keep the patch's current behavior, given we are already
inconsistent about what "-i" means.

i.e. createuser tries taking either $PGUSER or the current username as
a default user to create, while dropuser just bails out. Personally, I
prefer just bailing out if no create/drop user is specified, but
either way I think they should be consistent.

That is intentional long-standing behavior.  createdb/dropdb work the
same way.

OK.

Josh