Обсуждение: BUG #15025: PSQL CLI - inconsistency when both -d and -U supplies ausername
BUG #15025: PSQL CLI - inconsistency when both -d and -U supplies ausername
От
PG Bug reporting form
Дата:
The following bug has been logged on the website:
Bug reference: 15025
Logged by: Akos Vandra
Email address: akos@elegran.com
PostgreSQL version: 9.5.4
Operating system: Debian
Description:
Repro:
case 1: psql -U other_user -d "postgresql://some_user@host/db"
case 2: psql -d "postgresql://some_user@host/db" -U other_user
Expectation:
Use whatever is given later:
case 1: log in as user
case 2: log in as other_user
Actual:
case 1: logs in as user
case 2:
- the password prompt asks for the pw of user
- psql uses the password given to log in with other_user
- if the pw is correct for user, or incorrect it displays that the
password is incorrect for other_user
- if the password is correct for other_user, it connects to the db as
other_user
$ psql "postgresql://user@host/db" -U other_user
Password for user other_user:
psql: FATAL: password authentication failed for user "user"
FATAL: password authentication failed for user "user"
Re: BUG #15025: PSQL CLI - inconsistency when both -d and -Usupplies a username
От
Bruce Momjian
Дата:
On Tue, Jan 23, 2018 at 01:00:13PM +0000, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference: 15025
> Logged by: Akos Vandra
> Email address: akos@elegran.com
> PostgreSQL version: 9.5.4
> Operating system: Debian
> Description:
>
> Repro:
>
> case 1: psql -U other_user -d "postgresql://some_user@host/db"
>
> case 2: psql -d "postgresql://some_user@host/db" -U other_user
>
> Expectation:
>
> Use whatever is given later:
>
> case 1: log in as user
> case 2: log in as other_user
>
> Actual:
>
> case 1: logs in as user
> case 2:
> - the password prompt asks for the pw of user
> - psql uses the password given to log in with other_user
> - if the pw is correct for user, or incorrect it displays that the
> password is incorrect for other_user
> - if the password is correct for other_user, it connects to the db as
> other_user
>
>
> $ psql "postgresql://user@host/db" -U other_user
> Password for user other_user:
> psql: FATAL: password authentication failed for user "user"
> FATAL: password authentication failed for user "user"
I was able to make a clearer example. First create two users:
CREATE USER user1 PASSWORD 'abc1';
CREATE USER user2 PASSWORD 'abc2';
then:
psql -d "postgresql://user2@momjian.us/test" -U user1
--> Password for user user1:
but it wants the user2 password. Same with:
psql -U user1 -d "postgresql://user2@momjian.us/test"
--> Password for user user1:
It doesn't matter whether -U is first or last, it always prompts for the
-U user, but connects as the -d user.
Because the URI is parsed by libpq, I don't think we can do any better
than just suppress the user name in the password prompt when a URI is
used. This is done in the attached patch, e.g.:
e.g.:
$psql -U user1 -d "postgresql://user2@momjian.us/test"
Password:
Can someone tell me if I need to update the prompt in
psql/command.c::prompt_for_password()? I can't figure out how to
trigger that prompt.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +
Вложения
Bruce Momjian <bruce@momjian.us> writes:
> psql -d "postgresql://user2@momjian.us/test" -U user1
> --> Password for user user1:
> but it wants the user2 password.
> It doesn't matter whether -U is first or last, it always prompts for the
> -U user, but connects as the -d user.
Bleah.
> Because the URI is parsed by libpq, I don't think we can do any better
> than just suppress the user name in the password prompt when a URI is
> used.
That doesn't seem very user-friendly at all. I think that the most
correct behavior in this case would be to throw an error because of the
conflicting command line parameters. Now admittedly, psql doesn't throw
an error for
psql -U alice -U bob
so maybe just silently making a choice is OK, but we need to be clear
as to which choice we made. Isn't it possible to get the URI parse
results back out of libpq?
regards, tom lane
Re: BUG #15025: PSQL CLI - inconsistency when both -d and -Usupplies a username
От
Bruce Momjian
Дата:
On Sun, Jan 28, 2018 at 02:38:46PM -0500, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > psql -d "postgresql://user2@momjian.us/test" -U user1 > > --> Password for user user1: > > but it wants the user2 password. > > > It doesn't matter whether -U is first or last, it always prompts for the > > -U user, but connects as the -d user. > > Bleah. > > > Because the URI is parsed by libpq, I don't think we can do any better > > than just suppress the user name in the password prompt when a URI is > > used. > > That doesn't seem very user-friendly at all. I think that the most > correct behavior in this case would be to throw an error because of the > conflicting command line parameters. Now admittedly, psql doesn't throw > an error for > > psql -U alice -U bob > > so maybe just silently making a choice is OK, but we need to be clear > as to which choice we made. Isn't it possible to get the URI parse > results back out of libpq? Well, there is PQuser(), but you need to pass a connection struct to that, and before you connect you don't have one. libpq's conninfo_uri_parse_options() is a static function so that can't be used. I don't see any libpq documentation saying that URIs override command-line specifications. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Bruce Momjian <bruce@momjian.us> writes:
> On Sun, Jan 28, 2018 at 02:38:46PM -0500, Tom Lane wrote:
>> Isn't it possible to get the URI parse
>> results back out of libpq?
> Well, there is PQuser(), but you need to pass a connection struct to
> that, and before you connect you don't have one.
Yeah, but we normally don't prompt for password till after a failed
connection attempt, at which point we can get the info. So I propose
something like the attached.
There's room for debate about what we ought to do when -W (--password) is
specified, but I think that that's not really that exciting because the
only real use-cases for it are noninteractive applications that aren't
going to care what the prompt is. So in the startup.c case I have it
just offering the neutral "Password: " prompt always. In the \c case,
I left it using the same initial username as it was before, because the
odds that that's right seem considerably higher with \c. You can still
fool it by giving a URI dbname to \c, so maybe there's an argument for
lobotomizing the initial prompt in \c too, but I didn't do that here.
regards, tom lane
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 015c391..63a6f99 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2829,7 +2829,7 @@ prompt_for_password(const char *username)
{
char buf[100];
- if (username == NULL)
+ if (username == NULL || username[0] == '\0')
simple_prompt("Password: ", buf, sizeof(buf), false);
else
{
@@ -2957,6 +2957,11 @@ do_connect(enum trivalue reuse_previous_specification,
* XXX: this behavior leads to spurious connection attempts recorded in
* the postmaster's log. But libpq offers no API that would let us obtain
* a password and then continue with the first connection attempt.
+ *
+ * XXX: prompting with "user" might be the wrong thing, if the dbname is a
+ * connstring or URI that overrides that. But getPassword == TRI_YES is a
+ * seldom-used option, so it doesn't seem worth sweating over. The normal
+ * prompting path below gets it right.
*/
if (pset.getPassword == TRI_YES)
{
@@ -3026,8 +3031,12 @@ do_connect(enum trivalue reuse_previous_specification,
*/
if (!password && PQconnectionNeedsPassword(n_conn) && pset.getPassword != TRI_NO)
{
+ /*
+ * Prompt for password using the username we actually connected
+ * with --- it might've come out of "dbname" rather than "user".
+ */
+ password = prompt_for_password(PQuser(n_conn));
PQfinish(n_conn);
- password = prompt_for_password(user);
continue;
}
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index ec6ae45..be57574 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -101,7 +101,6 @@ main(int argc, char *argv[])
int successResult;
bool have_password = false;
char password[100];
- char *password_prompt = NULL;
bool new_pass;
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("psql"));
@@ -205,15 +204,14 @@ main(int argc, char *argv[])
pset.popt.topt.recordSep.separator_zero = false;
}
- if (options.username == NULL)
- password_prompt = pg_strdup(_("Password: "));
- else
- password_prompt = psprintf(_("Password for user %s: "),
- options.username);
-
if (pset.getPassword == TRI_YES)
{
- simple_prompt(password_prompt, password, sizeof(password), false);
+ /*
+ * We can't be sure yet of the username that will be used, so don't
+ * offer a potentially wrong one. Typical uses of this option are
+ * noninteractive anyway.
+ */
+ simple_prompt("Password: ", password, sizeof(password), false);
have_password = true;
}
@@ -252,15 +250,28 @@ main(int argc, char *argv[])
!have_password &&
pset.getPassword != TRI_NO)
{
+ /*
+ * Before closing the old PGconn, extract the user name that was
+ * actually connected with --- it might've come out of a URI or
+ * connstring "database name" rather than options.username.
+ */
+ const char *realusername = PQuser(pset.db);
+ char *password_prompt;
+
+ if (realusername && realusername[0])
+ password_prompt = psprintf(_("Password for user %s: "),
+ realusername);
+ else
+ password_prompt = pg_strdup(_("Password: "));
PQfinish(pset.db);
+
simple_prompt(password_prompt, password, sizeof(password), false);
+ free(password_prompt);
have_password = true;
new_pass = true;
}
} while (new_pass);
- free(password_prompt);
-
if (PQstatus(pset.db) == CONNECTION_BAD)
{
fprintf(stderr, "%s: %s", pset.progname, PQerrorMessage(pset.db));
Re: BUG #15025: PSQL CLI - inconsistency when both -d and -Usupplies a username
От
Bruce Momjian
Дата:
On Sun, Jan 28, 2018 at 03:30:54PM -0500, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > On Sun, Jan 28, 2018 at 02:38:46PM -0500, Tom Lane wrote: > >> Isn't it possible to get the URI parse > >> results back out of libpq? > > > Well, there is PQuser(), but you need to pass a connection struct to > > that, and before you connect you don't have one. > > Yeah, but we normally don't prompt for password till after a failed > connection attempt, at which point we can get the info. So I propose > something like the attached. > > There's room for debate about what we ought to do when -W (--password) is > specified, but I think that that's not really that exciting because the > only real use-cases for it are noninteractive applications that aren't > going to care what the prompt is. So in the startup.c case I have it > just offering the neutral "Password: " prompt always. In the \c case, > I left it using the same initial username as it was before, because the > odds that that's right seem considerably higher with \c. You can still > fool it by giving a URI dbname to \c, so maybe there's an argument for > lobotomizing the initial prompt in \c too, but I didn't do that here. Oh, I wasn't aware a failed login left us with a 'conn'. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Bruce Momjian <bruce@momjian.us> writes:
> Oh, I wasn't aware a failed login left us with a 'conn'.
Sure, it's what's carrying the error message. But it's also got all
the actual connection parameters filled in.
After further experimentation it seems like the has_connection_string
logic in do_connect() causes the \c case to behave pretty much as you'd
want:
$ psql -U user1 -d "postgresql://user2@localhost/postgres" -W
Password:
postgres=> \c -
Password for user user2:
You are now connected to database "postgres" as user "user2".
postgres=> \c postgresql://user1@localhost/postgres
Password:
You are now connected to database "postgres" as user "user1".
So I now think the comment I added to do_connect() is unduly pessimistic,
and it's fine to keep using "prompt_for_password(user)" for a forced
password prompt there. There may still be use-cases where it gets it
wrong, but they're too narrow to be worth giving up the helpful prompt
altogether.
regards, tom lane
I wrote:
> So I now think the comment I added to do_connect() is unduly pessimistic,
> and it's fine to keep using "prompt_for_password(user)" for a forced
> password prompt there. There may still be use-cases where it gets it
> wrong, but they're too narrow to be worth giving up the helpful prompt
> altogether.
After further thought about that I changed my mind again: it seems better
to be able to say "we never issue a misleading password prompt" than that
"it's right 99% of the time" --- and it looks like in some cases with
nondefault reuse_previous_specification, we'd still get it wrong. So
I made it shorten the prompt if the dbname is a connstring or URI.
Committed with that change.
regards, tom lane